Skip to content
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

Fleet scale down: Remove < Ready GameServers first #2702

Merged
merged 1 commit into from
Aug 6, 2022

Conversation

markmandel
Copy link
Collaborator

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking

/kind bug

/kind cleanup
/kind documentation
/kind feature
/kind hotfix

What this PR does / Why we need it:

When scaling down GameServers within a single node, Agones would remove GameServers that would be there the longest.

This would mean that what would usually happen is that Ready GameServers would be shut down preferentially over GameServers that where not yet Ready.

This would result in churn on the node, and a drop in Ready GameServers until the newer ones came to a Ready state.

This change will prefer GameServers that are in a state before Ready for deletion, when comparing GameServers that are stored on a single node.

Which issue(s) this PR fixes:

Closes #2372

Special notes for your reviewer:

When scaling down GameServers within a single node, Agones would remove
GameServers that would be there the longest.

This would mean that what would usually happen is that Ready
GameServers would be shut down preferentially over GameServers that
where not yet Ready.

This would result in churn on the node, and a drop in Ready GameServers
until the newer ones came to a Ready state.

This change will prefer GameServers that are in a state before Ready for
deletion, when comparing GameServers that are stored on a single node.

Closes googleforgames#2372
@markmandel markmandel added kind/bug These are bugs. area/performance Anything to do with Agones being slow, or making it go faster. labels Aug 5, 2022
@markmandel
Copy link
Collaborator Author

Just as a demo, here is a local test I ran:

root@ca6c2ca7bb5f:/go/src/agones.dev/agones# kubectl apply -f ./examples/xonotic/fleet.yaml
fleet.agones.dev/xonotic created
root@ca6c2ca7bb5f:/go/src/agones.dev/agones# kubectl get gs
NAME                  STATE       ADDRESS        PORT   NODE                                     AGE
xonotic-z7pr7-dshlz   Scheduled   34.82.249.90   7157   gke-test-cluster-default-d5453681-86k9   2s
xonotic-z7pr7-gxm4c   Scheduled   34.82.249.90   7886   gke-test-cluster-default-d5453681-86k9   2s
root@ca6c2ca7bb5f:/go/src/agones.dev/agones# kubectl get gs
NAME                  STATE   ADDRESS        PORT   NODE                                     AGE
xonotic-z7pr7-dshlz   Ready   34.82.249.90   7157   gke-test-cluster-default-d5453681-86k9   14s
xonotic-z7pr7-gxm4c   Ready   34.82.249.90   7886   gke-test-cluster-default-d5453681-86k9   14s
root@ca6c2ca7bb5f:/go/src/agones.dev/agones# kubectl scale fleet xonotic --replicas=7
fleet.agones.dev/xonotic scaled
root@ca6c2ca7bb5f:/go/src/agones.dev/agones# kubectl get gs
NAME                  STATE       ADDRESS        PORT   NODE                                     AGE
xonotic-z7pr7-5whlq   Scheduled   34.82.249.90   7477   gke-test-cluster-default-d5453681-86k9   1s
xonotic-z7pr7-dshlz   Ready       34.82.249.90   7157   gke-test-cluster-default-d5453681-86k9   28s
xonotic-z7pr7-gxm4c   Ready       34.82.249.90   7886   gke-test-cluster-default-d5453681-86k9   28s
xonotic-z7pr7-hmznj   Scheduled   34.82.249.90   7789   gke-test-cluster-default-d5453681-86k9   1s
xonotic-z7pr7-llswl   Scheduled   34.82.249.90   7675   gke-test-cluster-default-d5453681-86k9   1s
xonotic-z7pr7-n7dpg   Scheduled   34.82.249.90   7093   gke-test-cluster-default-d5453681-86k9   1s
xonotic-z7pr7-w2x8d   Scheduled   34.82.249.90   7459   gke-test-cluster-default-d5453681-86k9   1s
root@ca6c2ca7bb5f:/go/src/agones.dev/agones# kubectl scale fleet xonotic --replicas=3
fleet.agones.dev/xonotic scaled
root@ca6c2ca7bb5f:/go/src/agones.dev/agones# kubectl get gs
NAME                  STATE       ADDRESS        PORT   NODE                                     AGE
xonotic-z7pr7-dshlz   Ready       34.82.249.90   7157   gke-test-cluster-default-d5453681-86k9   34s
xonotic-z7pr7-gxm4c   Ready       34.82.249.90   7886   gke-test-cluster-default-d5453681-86k9   34s
xonotic-z7pr7-hmznj   Shutdown    34.82.249.90   7789   gke-test-cluster-default-d5453681-86k9   7s
xonotic-z7pr7-llswl   Shutdown    34.82.249.90   7675   gke-test-cluster-default-d5453681-86k9   7s
xonotic-z7pr7-n7dpg   Shutdown    34.82.249.90   7093   gke-test-cluster-default-d5453681-86k9   7s
xonotic-z7pr7-w2x8d   Scheduled   34.82.249.90   7459   gke-test-cluster-default-d5453681-86k9   7s
root@ca6c2ca7bb5f:/go/src/agones.dev/agones# kubectl get gs
NAME                  STATE   ADDRESS        PORT   NODE                                     AGE
xonotic-z7pr7-dshlz   Ready   34.82.249.90   7157   gke-test-cluster-default-d5453681-86k9   37s
xonotic-z7pr7-gxm4c   Ready   34.82.249.90   7886   gke-test-cluster-default-d5453681-86k9   37s
xonotic-z7pr7-w2x8d   Ready   34.82.249.90   7459   gke-test-cluster-default-d5453681-86k9   10s

You can see - the Ready ones were left well enough alone! 🥳

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: c65ee955-963a-48a8-92df-cf87b1f9bcaa

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:

  • git fetch https://github.com/googleforgames/agones.git pull/2702/head:pr_2702 && git checkout pr_2702
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.tag=1.26.0-8ab3de4-amd64

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markmandel, roberthbailey

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [markmandel,roberthbailey]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot added the lgtm label Aug 6, 2022
@roberthbailey roberthbailey merged commit d741742 into googleforgames:main Aug 6, 2022
@markmandel markmandel deleted the bug/fleet-scale-down branch August 9, 2022 23:54
@SaitejaTamma SaitejaTamma added this to the 1.26.0 milestone Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/performance Anything to do with Agones being slow, or making it go faster. kind/bug These are bugs. lgtm size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove < Ready GameServers first when scaling down on the same node
4 participants