Skip to content

Fix race condition in PerNodeCounter by tracking processed events#4363

Merged
markmandel merged 2 commits intoagones-dev:mainfrom
markmandel:flaky/TestPerNodeCounterRun
Dec 1, 2025
Merged

Fix race condition in PerNodeCounter by tracking processed events#4363
markmandel merged 2 commits intoagones-dev:mainfrom
markmandel:flaky/TestPerNodeCounterRun

Conversation

@markmandel
Copy link
Copy Markdown
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
/kind release

What this PR does / Why we need it:

Introduce a 'processed' struct to track GameServer state by UID, resource version, state, and node name. This prevents duplicate events from being counted multiple times, fixing a race condition that caused flaky tests.

The processed map ensures that Add/Modify events for the same GameServer are only counted once if they represent the same state.

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

Flaky tests found a bug! That is what they do sometimes!

This is a bug that has been around for years but is probably small and rare enough that it didn't impact very much, but probably caused some people to scratch their heads occasionally on node packing. Now we'll get accurate node counts!

Introduce a 'processed' struct to track GameServer state by UID,
resource version, state, and node name. This prevents duplicate events
from being counted multiple times, fixing a race condition that caused
flaky tests.

The processed map ensures that Add/Modify events for the same GameServer
are only counted once if they represent the same state.
@github-actions github-actions bot added the kind/bug These are bugs. label Nov 30, 2025
@agones-bot
Copy link
Copy Markdown
Collaborator

Build Succeeded 🥳

Build Id: ce678b60-3738-4d39-919b-b09910bb9042

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/4363/head:pr_4363 && git checkout pr_4363
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.54.0-dev-f62d9c8

Copy link
Copy Markdown
Collaborator

@lacroixthomas lacroixthomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@markmandel markmandel enabled auto-merge (squash) December 1, 2025 01:08
@agones-bot
Copy link
Copy Markdown
Collaborator

Build Succeeded 🥳

Build Id: 6916c410-67d1-4214-8057-da4d8604ca94

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/4363/head:pr_4363 && git checkout pr_4363
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.54.0-dev-ca527aa

@markmandel markmandel merged commit 3b94c58 into agones-dev:main Dec 1, 2025
4 checks passed
@markmandel markmandel deleted the flaky/TestPerNodeCounterRun branch December 1, 2025 02:10
mnthe pushed a commit to mnthe/agones that referenced this pull request Mar 23, 2026
…ones-dev#4363)

Introduce a 'processed' struct to track GameServer state by UID,
resource version, state, and node name. This prevents duplicate events
from being counted multiple times, fixing a race condition that caused
flaky tests.

The processed map ensures that Add/Modify events for the same GameServer
are only counted once if they represent the same state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug These are bugs. size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants