Allocation: Re-cache allocated `GameServer` #4159

markmandel · 2025-04-17T03:47:42Z

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
/kind release

What this PR does / Why we need it:

We want the allocated GameServer back in the cache anyway for re-allocation, so might as well get it back in as soon as possible.

Which issue(s) this PR fixes:

Work on #3992

Special notes for your reviewer:

agones-bot · 2025-04-17T04:45:18Z

Build Succeeded 🥳

Build Id: 800070be-37f3-4e69-b24f-15f108a1ac20

The following development artifacts have been built, and will exist for the next 30 days:

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.49.0-dev-2ef2d5e
image: us-docker.pkg.dev/agones-images/ci/agones-extensions:1.49.0-dev-2ef2d5e
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.49.0-dev-2ef2d5e-linux
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.49.0-dev-2ef2d5e
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.49.0-dev-2ef2d5e
Linux C++ SDK (build): agonessdk-1.49.0-dev-2ef2d5e-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.49.0-dev-2ef2d5e.zip

A preview of the website (the last 30 builds are retained):

https://2ef2d5e-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/4159/head:pr_4159 && git checkout pr_4159
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.49.0-dev-2ef2d5e

igooch · 2025-04-21T22:25:20Z

@dmorgan-blizz FYI this is one proposal for your issue #3992. Let us know how this performs.

markmandel · 2025-04-22T01:29:26Z

@dmorgan-blizz FYI this is one proposal for your issue #3992. Let us know how this performs.

There should be no downside to this change, it should help no matter what.

markmandel · 2025-04-23T19:32:03Z

Any blocking reason this shouldn't be merged?

agones-bot · 2025-04-23T21:32:05Z

Build Succeeded 🥳

Build Id: 2c277f71-ca90-4eb7-9ef5-a04a815ff008

The following development artifacts have been built, and will exist for the next 30 days:

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.49.0-dev-d042a1d
image: us-docker.pkg.dev/agones-images/ci/agones-extensions:1.49.0-dev-d042a1d
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.49.0-dev-d042a1d-linux
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.49.0-dev-d042a1d
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.49.0-dev-d042a1d
Linux C++ SDK (build): agonessdk-1.49.0-dev-d042a1d-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.49.0-dev-d042a1d.zip

A preview of the website (the last 30 builds are retained):

https://d042a1d-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/4159/head:pr_4159 && git checkout pr_4159
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.49.0-dev-d042a1d

peterzhongyi

Should we worry about the informer giving a stale update on the game server that overwrites the game server record you write here? Seems like it might happen frequently given that the informer is almost always going to be slower but will still try to write the the cache?

markmandel · 2025-04-24T22:11:21Z

🤔 I can't think of an example, since the Informer is only going to give you the latest version of the GameServer instance.

We could do a generation check on store, and if it's less than the latest version that is stored in there (which is possibly not a bad idea anyway), we reject it.

agones/pkg/gameserverallocations/cache.go

Lines 32 to 38 in daafb8f

    
           func (e *gameServerCache) Store(key string, gs *agonesv1.GameServer) { 
        
           	e.mu.Lock() 
        
           	defer e.mu.Unlock() 
        
           	if e.cache == nil { 
        
           		e.cache = map[string]*agonesv1.GameServer{} 
        
           	} 
        
           	e.cache[key] = gs.DeepCopy()

Actually, that's a pretty good idea anyway, I'll put that in 👍🏻 seems like thing that shouldn't happen often (ever?), but easy enough to make sure it doesn't.

We want the allocated `GameServer` back in the cache anyway for re-allocation, so might as well get it back in as soon as possible. Work on googleforgames#3992

peterzhongyi · 2025-04-28T14:54:58Z

Sounds good. LGTM!

agones-bot · 2025-04-28T15:51:36Z

Build Succeeded 🥳

Build Id: 9a44c0be-ec98-48a9-9c04-0b3be35b39e6

The following development artifacts have been built, and will exist for the next 30 days:

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.49.0-dev-c1199d6
image: us-docker.pkg.dev/agones-images/ci/agones-extensions:1.49.0-dev-c1199d6
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.49.0-dev-c1199d6-linux
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.49.0-dev-c1199d6
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.49.0-dev-c1199d6
Linux C++ SDK (build): agonessdk-1.49.0-dev-c1199d6-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.49.0-dev-c1199d6.zip

A preview of the website (the last 30 builds are retained):

https://c1199d6-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/4159/head:pr_4159 && git checkout pr_4159
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.49.0-dev-c1199d6

markmandel · 2025-04-29T03:37:56Z

pkg/gameserverallocations/cache.go

+	if existingGS, ok := e.cache[key]; ok {
+		// If the incoming gs object's Generation is less than the existing one, ignore it, but also check the ResourceVersion
+		// saves us a deep copy if we don't need to
+		if gs.ObjectMeta.Generation < existingGS.ObjectMeta.Generation || gs.ObjectMeta.ResourceVersion == existingGS.ObjectMeta.ResourceVersion {


I had forgotten that you can get resourceVersion change, but with no Generation change, so you have to account for it, and lean on eventual consistency when it's needed.

agones-bot · 2025-04-29T04:17:27Z

Build Failed 😭

Build Id: 7aebfd4a-d18d-4d04-9bd5-ebe1ec51fb2e

Status: FAILURE

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2025-04-30T18:39:41Z

Build Succeeded 🥳

Build Id: f4d08f7b-4d6a-4cfd-bb5c-144be473a4a0

The following development artifacts have been built, and will exist for the next 30 days:

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.49.0-dev-831a8e2
image: us-docker.pkg.dev/agones-images/ci/agones-extensions:1.49.0-dev-831a8e2
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.49.0-dev-831a8e2-linux
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.49.0-dev-831a8e2
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.49.0-dev-831a8e2
Linux C++ SDK (build): agonessdk-1.49.0-dev-831a8e2-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.49.0-dev-831a8e2.zip

A preview of the website (the last 30 builds are retained):

https://831a8e2-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/4159/head:pr_4159 && git checkout pr_4159
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.49.0-dev-831a8e2

markmandel requested review from igooch and peterzhongyi April 17, 2025 03:47

github-actions bot added the kind/bug These are bugs. label Apr 17, 2025

markmandel changed the title ~~Allocation: Re-cache allocated GameSerer~~ Allocation: Re-cache allocated GameServer Apr 17, 2025

markmandel mentioned this pull request Apr 17, 2025

Creates Feature Flag to Prevent Removal of an Allocated Game Server from the Allocation Cache #4160

Closed

markmandel added the area/performance Anything to do with Agones being slow, or making it go faster. label Apr 18, 2025

peterzhongyi reviewed Apr 24, 2025

View reviewed changes

Allocation: Re-cache allocated GameSerer

dbd3e27

We want the allocated `GameServer` back in the cache anyway for re-allocation, so might as well get it back in as soon as possible. Work on googleforgames#3992

peterzhongyi approved these changes Apr 28, 2025

View reviewed changes

Add cache generation check.

9eed0e6

markmandel force-pushed the perf/re-allocation branch from c1199d6 to 9eed0e6 Compare April 29, 2025 03:36

markmandel commented Apr 29, 2025

View reviewed changes

Merge branch 'main' into perf/re-allocation

831a8e2

markmandel merged commit 4b0c6c2 into googleforgames:main Apr 30, 2025
4 checks passed

markmandel deleted the perf/re-allocation branch April 30, 2025 20:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allocation: Re-cache allocated `GameServer` #4159

Allocation: Re-cache allocated `GameServer` #4159

Uh oh!

markmandel commented Apr 17, 2025

Uh oh!

agones-bot commented Apr 17, 2025

Uh oh!

igooch commented Apr 21, 2025

Uh oh!

markmandel commented Apr 22, 2025

Uh oh!

markmandel commented Apr 23, 2025

Uh oh!

agones-bot commented Apr 23, 2025

Uh oh!

peterzhongyi left a comment

Uh oh!

markmandel commented Apr 24, 2025

Uh oh!

peterzhongyi commented Apr 28, 2025

Uh oh!

agones-bot commented Apr 28, 2025

Uh oh!

markmandel Apr 29, 2025

Uh oh!

agones-bot commented Apr 29, 2025

Uh oh!

agones-bot commented Apr 30, 2025

Uh oh!

Uh oh!

Uh oh!

Allocation: Re-cache allocated GameServer #4159

Allocation: Re-cache allocated GameServer #4159

Uh oh!

Conversation

markmandel commented Apr 17, 2025

Uh oh!

agones-bot commented Apr 17, 2025

Uh oh!

igooch commented Apr 21, 2025

Uh oh!

markmandel commented Apr 22, 2025

Uh oh!

markmandel commented Apr 23, 2025

Uh oh!

agones-bot commented Apr 23, 2025

Uh oh!

peterzhongyi left a comment

Choose a reason for hiding this comment

Uh oh!

markmandel commented Apr 24, 2025

Uh oh!

peterzhongyi commented Apr 28, 2025

Uh oh!

agones-bot commented Apr 28, 2025

Uh oh!

markmandel Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

agones-bot commented Apr 29, 2025

Uh oh!

agones-bot commented Apr 30, 2025

Uh oh!

Uh oh!

Uh oh!

Allocation: Re-cache allocated `GameServer` #4159

Allocation: Re-cache allocated `GameServer` #4159