fix: stop handing matchmaker-owned GameServers back to the fleet#60
Merged
Conversation
A GameServer allocated by an external matchmaker is Allocated but empty for a few seconds — its players are still in flight (assign -> proxy -> client connect). Today the plugin reads that as "nobody here, I'm free": the readiness loop runs every 10s and calls ready() on an empty server, and the last-player-left listener does the same. Observed live: an external GameServerAllocation flipped a server to Allocated, and 4 seconds later the plugin logged "SDK.Ready() complete" and put it back in the Ready pool. The next allocation would then hand the same server to a second match — two matches on one server, player visible. The reaper cannot catch it either, because it hunts *Allocated* servers with a stale match-id and this orphan is Ready. So: when a matchmaker owns the lifecycle, keep hands off the state. New GameServerOwnership reads GROUNDS_MATCHMAKING (which forge will set on the Fleet template for gamemodes that declare a matchmaking: block) and, when set, disables both the readiness loop and the player listeners on paper and minestom. Ending the match stays the gamemode's job (SDK.Shutdown), so the server never re-enters the pool and the fleet replaces it with a fresh one. An env var rather than reading our own GameServer labels: the match-id label is patched on at allocation time, so a server polling for it would race the very window this closes. The env var is set at pod creation. Unparseable values stay SELF_MANAGED — failing open would strand an ordinary lobby outside the Ready pool and make it unjoinable. Lobbies and standalone gamemodes are untouched: without the env var the behaviour is exactly what it is today.
|
hbrombeer
added a commit
that referenced
this pull request
Jul 13, 2026
…sible (#61) Matchmaker-managed servers never entered the pool. Agones only moves a GameServer out of Scheduled when it calls ready(), and #60 removed the call entirely — so the fleet sat there looking perfectly healthy while, to an allocator, it did not exist. Every allocation came back UnAllocated and no match could ever be placed. Verified live: the duel fleet's GameServer stayed in Starting indefinitely. Keeping our hands off the state was too broad a reading. What must not happen is re-readying a server that is ALREADY allocated — its players are seconds away, and handing it back lets a second match land on it. That is the bug #60 fixed, and it stays fixed: no readiness loop, no player listeners, no ready() on empty. So the server readies exactly once, to enter the pool, and never touches its state again. Ending the match remains the gamemode's job. The KDoc that said "no ready()" is corrected too — it is what drove the mistake.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the blocker found while spiking cross-cluster allocation for service-match.
The bug
A GameServer allocated by an external matchmaker is
Allocatedbut empty for a few seconds — the players are still on their way in (assign → proxy → client connect). The plugin currently reads "empty" as "free":scheduleFallback()runs every 10 s and forcesonlinePlayers.isEmpty() ? ready() : allocate()ready()tooObserved live (spike against
vcluster-dahendriik): an externalGameServerAllocationflipped a server toAllocated, and 4 seconds later the plugin loggedSDK.Ready() completeand returned it to the Ready pool. The fleet then reportsALLOCATED=0.Consequences:
grounds/match-idis stale, and this orphan isReady.This also answers an open question in the design doc ("does the gamemode base idle-shut-down when empty?") — it does the opposite.
The fix
New
GameServerOwnershipincommon, read fromGROUNDS_MATCHMAKING. When set, the plugin keeps its hands off the Agones state on both paper and minestom: no readiness loop, no player listeners, noready(). Ending the match stays the gamemode's job (SDK.Shutdown), so the server never re-enters the pool and the fleet replaces it with a fresh one.Why an env var and not our own GameServer labels: the
grounds/match-idlabel is patched on at allocation time, so a server polling for it would race the very window this is meant to close. The env var is set at pod creation — the server knows from birth which mode it is in.Fails safe, not open: an unparseable value stays
SELF_MANAGED. Silently disabling the readiness loop on an ordinary lobby would strand it outside the Ready pool and make it unjoinable.Blast radius
None for anything running today. Without
GROUNDS_MATCHMAKING, behaviour is byte-for-byte what it is now — lobbies and standalone gamemodes keep managing themselves, which is correct for them ("empty" really does mean "free" there).Verification
1/true/ garbage), green./gradlew buildgreen; paper and minestom both compileDraft: forge still needs to render
GROUNDS_MATCHMAKINGonto the Fleet template for gamemodes with amatchmaking:block. Nothing consumes the flag until then, so this is safe to merge ahead of it.