-
Notifications
You must be signed in to change notification settings - Fork 817
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
Refactor AllocationCounter to GameServerCounter #639
Refactor AllocationCounter to GameServerCounter #639
Conversation
Build Succeeded 👏 Build Id: 72d09add-848a-421e-8bb5-103beabc7d07 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/gameservers/gameservercounter.go
Outdated
@@ -166,7 +169,7 @@ func (ac *AllocationCounter) Run(stop <-chan struct{}) error { | |||
} | |||
|
|||
// Counts returns the NodeCount map in a thread safe way | |||
func (ac *AllocationCounter) Counts() map[string]NodeCount { | |||
func (ac *GameServerCounter) Counts() map[string]NodeCount { |
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.
Nit: same as in the previous PR
func (ac *GameServerCounter) Counts() map[string]NodeCount { | |
func (gc *GameServerCounter) Counts() map[string]NodeCount { |
pkg/gameservers/gameservercounter.go
Outdated
@@ -29,11 +29,11 @@ import ( | |||
"k8s.io/client-go/tools/cache" | |||
) | |||
|
|||
// AllocationCounter counts how many Allocated and | |||
// GameServerCounter counts how many Allocated and |
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.
nit: can we call it PerNodeCounter or something likethat? this is to avoid stuttering "gameservers.GameServerCounter", instead "gameservers.PerNodeCounter"
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.
I like it. That reads much better. Making this change!
21127c6
to
a5cd54d
Compare
Build Succeeded 👏 Build Id: 6b024e51-34ba-4421-a023-e37b67249d2d 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:
|
Precursor to googleforgames#638 - this moves the AllocationCounter code to a more central GameServerCounter (which I think is a better name) that tracks the number of Ready and Allocated GameServers that are available on each node. These details are useful for sorting for `Packed` scheduling strategies. Once this PR is completed, we can use this Count() values provided by this controller in the GameServerSet scale down logic, to ensure that GameServers on the least used nodes are removed first when using a Packed strategy.
a5cd54d
to
be0718d
Compare
Build Succeeded 👏 Build Id: 7c65795d-2107-4e45-bee5-3037a0e7e9d8 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:
|
Precursor to #638 - this moves the AllocationCounter code to a more central GameServerCounter (which I think is a better name) that tracks the number of Ready and Allocated GameServers that are
available on each node.
These details are useful for sorting for
Packed
scheduling strategies.Once this PR is completed, we can use this Count() values provided by this controller in the GameServerSet scale down logic, to ensure that GameServers on the least used nodes are removed first when using a Packed strategy.