-
Notifications
You must be signed in to change notification settings - Fork 820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementation of SDK.Allocate() #721
Implementation of SDK.Allocate() #721
Conversation
/cc @victor-prodan finally 😄 |
Build Failed 😱 Build Id: 5f8abd40-bf04-4b6a-906d-d85dc5de2030 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: d21e965e-576c-409e-9edb-0cc6937feda4 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
pkg/sdkserver/sdkserver.go
Outdated
// complete the operation due to contention issues. | ||
func (s *SDKServer) Allocate(context.Context, *sdk.Empty) (*sdk.Empty, error) { | ||
s.logger.Info("Received self Allocate request") | ||
err := wait.PollImmediate(500*time.Millisecond, 30*time.Second, func() (done bool, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait.ExponentialBackoff() is more appropriate to handle contention issues
having said that, I don't see why this would ever be contending - the GS is running so unlikely to be Unhealthy and not in Shutdown and not deleted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YES! You are right.
I was worried about the Exponential backoff potentially going for a really long time, but I can provide a Backoff with a Cap
, and I get the same thing. I will make this change. Nice catch - I was looking for this, but didn't find it previously 👍
pkg/sdkserver/sdkserver.go
Outdated
|
||
// try again, with some jitter | ||
if k8serrors.IsConflict(err) { | ||
jitter := wait.Jitter(500*time.Millisecond, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exponential back-off will do the sleep for you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, modulo perhaps switching backoff strategy
Build Succeeded 👏 Build Id: 2d05c19e-08e6-4b17-8de0-a17207dea4d3 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Now GameServers can self Allocate! This is just the implementation of the GO SDK at this stage (although the gRPC libraries have been regenerated). Other languages can come in later PRs. This is the first part of 1st Party MatchMaking support (googleforgames#660)
4932547
to
2f8d3ae
Compare
Build Succeeded 👏 Build Id: 807462c4-1210-4c9c-a3e8-fb0e1e6c2188 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Now GameServers can self Allocate!
This is just the implementation of the GO SDK at this stage (although the gRPC libraries have been regenerated). Other languages can come in later PRs.
This is the first part of 1st Party MatchMaking support (#660)