From 04c484ac3d2cc5f116d868b033da6a4a85c31a62 Mon Sep 17 00:00:00 2001 From: Hendrik Brombeer <20863417+hbrombeer@users.noreply.github.com> Date: Sat, 17 Oct 2026 17:33:23 +0200 Subject: [PATCH 1/2] feat(velocity)!: fill the fullest lobby below a 100-player cap Least-occupied-first and the fleet's one-lobby Ready buffer put every player in a lobby of their own: the empty spare was always the least occupied, the first join allocated it, the autoscaler started another empty one, and the next join took that. Joins, lobby kicks and outdated-lobby moves now go to the fullest lobby below GROUNDS_LOBBY_PLAYER_CAP (default 100), and to an emptier one only when every lobby is at the cap. Counts are the larger of the network count and this proxy's own, plus this proxy's open reservations: each pick reserves a place for the player under one lock until they reach a server, leave, or 15s pass, so a burst of joins on one proxy cannot overfill a lobby. Across proxies the cap stays soft by the network count's lag. The plugin instance exposes pickLobby(UUID, String?) with JDK types so plugin-match can place returning players with the same rule and the same reservations, by reflection. BREAKING CHANGE: lobbies fill fullest-first instead of evenly. --- README.md | 7 ++ .../kotlin/gg/grounds/GroundsPluginAgones.kt | 22 +++- .../gg/grounds/discovery/DiscoveryConfig.kt | 6 + .../discovery/DiscoveryPlayerListener.kt | 10 +- .../gg/grounds/discovery/DiscoveryService.kt | 42 ++++--- .../gg/grounds/discovery/LobbyKickListener.kt | 10 +- .../gg/grounds/discovery/LobbyPlacement.kt | 97 +++++++++++++++ .../gg/grounds/discovery/LobbySelection.kt | 40 ++++--- .../grounds/discovery/OutdatedLobbyMover.kt | 15 ++- .../grounds/discovery/DiscoveryConfigTest.kt | 17 +++ .../grounds/discovery/DiscoveryServiceTest.kt | 10 +- .../discovery/LobbyKickListenerTest.kt | 11 +- .../grounds/discovery/LobbyPlacementTest.kt | 110 ++++++++++++++++++ .../grounds/discovery/LobbySelectionTest.kt | 73 +++++++++--- .../discovery/OutdatedLobbyMoverTest.kt | 5 +- 15 files changed, 400 insertions(+), 75 deletions(-) create mode 100644 velocity/src/main/kotlin/gg/grounds/discovery/LobbyPlacement.kt create mode 100644 velocity/src/test/kotlin/gg/grounds/discovery/LobbyPlacementTest.kt diff --git a/README.md b/README.md index 8068ec0..fc40b65 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,7 @@ deployments do not need to set anything. | `GROUNDS_AGONES_ADDRESS_TYPE` | `PodIP` | Which entry of `status.addresses` to dial (`PodIP`, `ExternalIP`, …) | | `GROUNDS_AGONES_PORT` | `25565` | TCP port on the GameServer | | `GROUNDS_STATIC_SERVERS` | _(none)_ | Comma-separated `name=host:port` static Velocity backends | +| `GROUNDS_LOBBY_PLAYER_CAP` | `100` | Players a lobby is filled to before joins go to the next one | `GROUNDS_STATIC_SERVERS` is validated strictly: names and hosts must be non-empty, ports must be between `1` and `65535`, and names must be unique. For example, Stage proxies can use: @@ -48,6 +49,12 @@ GROUNDS_STATIC_SERVERS=buildserver=buildserver:25565 This only registers the backend with Velocity; configuring the backend itself for Velocity forwarding remains a separate deployment concern. +Joins, lobby kicks and plugin-match's returns from a match all go to the fullest lobby that is +below `GROUNDS_LOBBY_PLAYER_CAP`, and to an emptier one only when every lobby is at the cap. +Counts are the larger of the network-wide count (plugin-proxy) and this proxy's own, plus the +players this proxy has sent but who have not arrived yet, so a burst of joins on one proxy cannot +overfill a lobby. Across proxies the cap is soft by the couple of seconds the network count lags. + Typical Helm chart wiring uses a `ConfigMap` consumed via `envFrom`, plus `POD_NAMESPACE` from the Downward API for clusters where the proxy should watch its own namespace: diff --git a/velocity/src/main/kotlin/gg/grounds/GroundsPluginAgones.kt b/velocity/src/main/kotlin/gg/grounds/GroundsPluginAgones.kt index 339c182..7ff1d2c 100644 --- a/velocity/src/main/kotlin/gg/grounds/GroundsPluginAgones.kt +++ b/velocity/src/main/kotlin/gg/grounds/GroundsPluginAgones.kt @@ -22,6 +22,7 @@ import gg.grounds.i18n.Translations import gg.grounds.presentation.VelocitySystemMessages import gg.grounds.proxy.api.ProxyServiceRegistry import gg.grounds.proxy.api.ServerDisplayQuery +import java.util.UUID import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -55,7 +56,7 @@ constructor(private val proxyServer: ProxyServer, private val logger: Logger) { ) val discoveryConfig = DiscoveryConfig.fromEnv() logger.info( - "Loaded Agones discovery config (namespace={}, labelSelector={}, lobbyLabel={}, lobbyValue={}, runningStates={}, pollInterval={}s, addressType={}, port={})", + "Loaded Agones discovery config (namespace={}, labelSelector={}, lobbyLabel={}, lobbyValue={}, runningStates={}, pollInterval={}s, addressType={}, port={}, lobbyPlayerCap={})", discoveryConfig.namespace, discoveryConfig.labelSelector.ifEmpty { "" }, discoveryConfig.lobbyLabel.ifEmpty { "" }, @@ -64,6 +65,7 @@ constructor(private val proxyServer: ProxyServer, private val logger: Logger) { discoveryConfig.pollInterval.toSeconds(), discoveryConfig.addressType, discoveryConfig.port, + discoveryConfig.lobbyPlayerCap, ) stateManager = @@ -117,7 +119,7 @@ constructor(private val proxyServer: ProxyServer, private val logger: Logger) { { serverName -> discoveryService.getServerRole(serverName) == discoveryConfig.lobbyValue }, - { exclude -> discoveryService.pickLobby(exclude) }, + { exclude, playerId -> discoveryService.pickLobby(exclude, playerId) }, ), ) outdatedLobbyMover = @@ -127,6 +129,7 @@ constructor(private val proxyServer: ProxyServer, private val logger: Logger) { logger, discoveryService::lobbyCandidates, { drainManager.isDraining }, + lobbyPlayerCap = discoveryConfig.lobbyPlayerCap, messages = messages, ) .also { it.start() } @@ -156,6 +159,21 @@ constructor(private val proxyServer: ProxyServer, private val logger: Logger) { coroutineScope.cancel() } + /** + * The lobby [playerId] should go to, never [exclude], with their place reserved; null when + * there is none or discovery has not started. + * + * For plugin-match, which sends players back to a lobby after a match. It calls this by + * reflection — JDK types only — so the two plugins share one placement rule and one set of + * reservations without a compile-time dependency on each other. + */ + fun pickLobby(playerId: UUID, exclude: String?): String? = + if (this::discoveryService.isInitialized) { + discoveryService.pickLobby(exclude, playerId)?.serverInfo?.name + } else { + null + } + /** * Publishes the server display query, if there is a proxy API to publish it into. * diff --git a/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryConfig.kt b/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryConfig.kt index 2238227..70b504b 100644 --- a/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryConfig.kt +++ b/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryConfig.kt @@ -24,6 +24,8 @@ internal fun canonicalServerName(name: String): String = name.lowercase(Locale.R * - `GROUNDS_AGONES_PORT` — TCP port to dial on the discovered GameServer. * - `GROUNDS_STATIC_SERVERS` — Comma-separated `name=host:port` Velocity backends that are * registered without Agones discovery. + * - `GROUNDS_LOBBY_PLAYER_CAP` — Players a lobby is filled up to before joins go to the next one + * (default 100). Not `GROUNDS_AGONES_`-prefixed: plugin-match reads the same variable. */ data class StaticServer(val name: String, val host: String, val port: Int) @@ -37,6 +39,7 @@ data class DiscoveryConfig( val addressType: String, val port: Int, val staticServers: List, + val lobbyPlayerCap: Int = LobbySelection.DEFAULT_PLAYER_CAP, ) { companion object { const val DEFAULT_NAMESPACE = "games" @@ -68,6 +71,9 @@ data class DiscoveryConfig( addressType = env["GROUNDS_AGONES_ADDRESS_TYPE"] ?: DEFAULT_ADDRESS_TYPE, port = env["GROUNDS_AGONES_PORT"]?.toIntOrNull() ?: DEFAULT_PORT, staticServers = parseStaticServers(env["GROUNDS_STATIC_SERVERS"]), + lobbyPlayerCap = + env["GROUNDS_LOBBY_PLAYER_CAP"]?.trim()?.toIntOrNull()?.takeIf { it > 0 } + ?: LobbySelection.DEFAULT_PLAYER_CAP, ) private val DURATION_PATTERN = Regex("""^(\d+)\s*(s|m|h)$""") diff --git a/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryPlayerListener.kt b/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryPlayerListener.kt index 6c41037..6b10e01 100644 --- a/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryPlayerListener.kt +++ b/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryPlayerListener.kt @@ -50,10 +50,10 @@ class DiscoveryPlayerListener( private val proxyServer: ProxyServer, /** * The lobby a joining player lands on, or null when there is none — see - * [DiscoveryService.pickLobby]. Spreads by network-wide counts when the network can be asked, - * by this proxy's own view otherwise, and avoids outdated lobbies. + * [DiscoveryService.pickLobby]. Fills the fullest lobby below the cap and avoids outdated + * lobbies. Given a player it reserves their place; given null it only asks. */ - private val pickLobby: () -> RegisteredServer?, + private val pickLobby: (UUID?) -> RegisteredServer?, private val serverRole: (String) -> String?, private val drainTransferCookie: DrainTransferCookie = DrainTransferCookie(), private val sourceCookiePending: (String) -> Boolean = { false }, @@ -70,7 +70,7 @@ class DiscoveryPlayerListener( fun onLogin(event: LoginEvent) { if ( !shouldDenyInitialLogin( - pickLobby() != null, + pickLobby(null) != null, proxyServer.allServers.any { server -> serverRole(server.serverInfo.name) == STATIC_SERVER_ROLE }, @@ -144,7 +144,7 @@ class DiscoveryPlayerListener( event.setInitialServer(preferred) return } - pickLobby()?.let(event::setInitialServer) + pickLobby(event.player.uniqueId)?.let(event::setInitialServer) } private inner class PendingCookieRequest( diff --git a/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryService.kt b/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryService.kt index f835c9e..7e31887 100644 --- a/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryService.kt +++ b/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryService.kt @@ -15,6 +15,7 @@ import io.kubernetes.client.openapi.apis.CoreV1Api import io.kubernetes.client.openapi.apis.CustomObjectsApi import io.kubernetes.client.util.Config import java.net.InetSocketAddress +import java.util.UUID import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.TimeUnit import org.slf4j.Logger @@ -80,6 +81,7 @@ class DiscoveryService( private val staticServerNames = config.staticServers.mapTo(mutableSetOf()) { canonicalServerName(it.name) } @Volatile private var countsSnapshot: Pair?>? = null + private val placement = LobbyPlacement(config.lobbyPlayerCap) fun start() { unregisterBakedInPlaceholders() @@ -109,8 +111,29 @@ class DiscoveryService( owningGameServerSets[canonicalServerName(serverName)]?.let { it in retiredGameServerSets } ?: false - /** Every registered lobby, with its player count and whether it is outdated. */ - fun lobbyCandidates(): List { + /** + * Every registered lobby, with its player count and whether it is outdated. The count includes + * the players this proxy is still sending there; see [LobbyPlacement]. + */ + fun lobbyCandidates(): List = + placement.withReservations(observedLobbies()) + + /** + * The lobby [playerId] should be sent to, never [exclude]; see [LobbySelection.pick]. With a + * [playerId] the place is reserved until they arrive; without one this only asks. + */ + fun pickLobby(exclude: String? = null, playerId: UUID? = null): RegisteredServer? { + val chosen = placement.place(observedLobbies(), playerId, exclude) ?: return null + return proxyServer.getServer(chosen).orElse(null) + } + + /** + * Every registered lobby with the players already on it. The network count says how many are on + * it across every proxy, but only once their sessions are written and the cache has refreshed; + * this proxy's own `playersConnected` is exact the moment a connection completes. The larger of + * the two is the better answer for both — a lag in either only ever hides players. + */ + private fun observedLobbies(): List { val lobbies = proxyServer.allServers.filter { it.serverInfo.name in lobbyServers } if (lobbies.isEmpty()) return emptyList() val counts = networkCountsCached() @@ -118,22 +141,12 @@ class DiscoveryService( val name = server.serverInfo.name LobbySelection.Candidate( name, - if (counts != null) counts[name] ?: 0 else server.playersConnected.size, + maxOf(counts?.get(name) ?: 0, server.playersConnected.size), isOutdated(name), ) } } - /** The lobby a player should be sent to, never [exclude]; see [LobbySelection.pick]. */ - fun pickLobby(exclude: String? = null): RegisteredServer? { - val candidates = - lobbyCandidates().filter { - exclude == null || canonicalServerName(it.name) != canonicalServerName(exclude) - } - val chosen = LobbySelection.pick(candidates) ?: return null - return proxyServer.getServer(chosen).orElse(null) - } - private fun unregisterBakedInPlaceholders() { for (server in proxyServer.allServers.filter(::isBakedInPlaceholder)) { proxyServer.unregisterServer(server.serverInfo) @@ -172,12 +185,13 @@ class DiscoveryService( } private fun registerListeners() { + proxyServer.eventManager.register(plugin, placement.Listener()) proxyServer.eventManager.register( plugin, DiscoveryPlayerListener( plugin, proxyServer, - { pickLobby() }, + { playerId -> pickLobby(playerId = playerId) }, this::getServerRole, drainTransferCookie, sourceCookiePending, diff --git a/velocity/src/main/kotlin/gg/grounds/discovery/LobbyKickListener.kt b/velocity/src/main/kotlin/gg/grounds/discovery/LobbyKickListener.kt index 9493c4e..d0ae331 100644 --- a/velocity/src/main/kotlin/gg/grounds/discovery/LobbyKickListener.kt +++ b/velocity/src/main/kotlin/gg/grounds/discovery/LobbyKickListener.kt @@ -3,6 +3,7 @@ package gg.grounds.discovery import com.velocitypowered.api.event.Subscribe import com.velocitypowered.api.event.player.KickedFromServerEvent import com.velocitypowered.api.proxy.server.RegisteredServer +import java.util.UUID /** * A lobby that goes away takes its players to another lobby instead of off the network. @@ -14,8 +15,11 @@ import com.velocitypowered.api.proxy.server.RegisteredServer */ class LobbyKickListener( private val isLobby: (String) -> Boolean, - /** Another lobby to send the player to, never the one named; null when there is none. */ - private val pickLobby: (exclude: String) -> RegisteredServer?, + /** + * Another lobby to send the player to, never the one named; null when there is none. Picked + * like a join — the fullest lobby with room — and reserved for the player. + */ + private val pickLobby: (exclude: String, playerId: UUID) -> RegisteredServer?, ) { @Subscribe @@ -24,7 +28,7 @@ class LobbyKickListener( if (event.result !is KickedFromServerEvent.DisconnectPlayer) return val kickedFrom = event.server.serverInfo.name if (!isLobby(kickedFrom)) return - val target = pickLobby(kickedFrom) ?: return + val target = pickLobby(kickedFrom, event.player.uniqueId) ?: return event.result = KickedFromServerEvent.RedirectPlayer.create(target, event.serverKickReason.orElse(null)) } diff --git a/velocity/src/main/kotlin/gg/grounds/discovery/LobbyPlacement.kt b/velocity/src/main/kotlin/gg/grounds/discovery/LobbyPlacement.kt new file mode 100644 index 0000000..9f3a9c0 --- /dev/null +++ b/velocity/src/main/kotlin/gg/grounds/discovery/LobbyPlacement.kt @@ -0,0 +1,97 @@ +package gg.grounds.discovery + +import com.velocitypowered.api.event.Subscribe +import com.velocitypowered.api.event.connection.DisconnectEvent +import com.velocitypowered.api.event.player.ServerPostConnectEvent +import java.time.Duration +import java.util.UUID + +/** + * Picks a lobby and remembers the pick until the player is on it. + * + * The counts a pick works from lag the joins it makes: a player this proxy just sent to a lobby is + * not in Velocity's `playersConnected` until the connection completes, and not in the network count + * until their session says so and the cache has refreshed. Without a memory of its own picks, a + * burst of joins would all see the same 99 and all go to the same lobby. + * + * So each pick reserves a place for the player, and a candidate's count is its observed count plus + * the reservations still open on it. The pick and the reservation happen under one lock, which + * makes joins on this proxy exact: the 101st sees 100 and goes elsewhere. A reservation ends when + * the player reaches a server (they are then in `playersConnected`), leaves the proxy, or after + * [ttl] — a connection that fails quietly must not hold a place for ever. + * + * Joins on *another* proxy are only seen through the network count, a couple of seconds late, so + * the cap is soft across proxies: two proxies filling the same lobby at once can overshoot it by + * the joins each makes in that window. That costs a few players over 100, not a lobby of 200. + */ +class LobbyPlacement( + private val cap: Int = LobbySelection.DEFAULT_PLAYER_CAP, + private val ttl: Duration = DEFAULT_TTL, + private val clock: () -> Long = System::nanoTime, +) { + private data class Reservation(val server: String, val expiresAt: Long) + + private val reservations = HashMap() + + /** + * The lobby for [playerId], never [exclude]; null when there is none. [observed] are the + * lobbies with the players already counted on them. A null [playerId] only asks — nothing is + * reserved. + */ + @Synchronized + fun place( + observed: List, + playerId: UUID? = null, + exclude: String? = null, + ): String? { + // A player placed again (kicked before arriving, say) must not count against themselves. + if (playerId != null) reservations.remove(playerId) + val candidates = + withReservations(observed).filter { + exclude == null || canonicalServerName(it.name) != canonicalServerName(exclude) + } + val chosen = LobbySelection.pick(candidates, cap) ?: return null + if (playerId != null) { + reservations[playerId] = + Reservation(canonicalServerName(chosen), clock() + ttl.toNanos()) + } + return chosen + } + + /** [observed] with this proxy's open reservations added to each lobby's count. */ + @Synchronized + fun withReservations(observed: List): List { + val now = clock() + reservations.values.removeIf { it.expiresAt - now <= 0 } + val pending = reservations.values.groupingBy { it.server }.eachCount() + return observed.map { + it.copy(players = it.players + (pending[canonicalServerName(it.name)] ?: 0)) + } + } + + @Synchronized + fun release(playerId: UUID) { + reservations.remove(playerId) + } + + /** Ends a reservation once its player is on a server, or gone. */ + inner class Listener { + // Post-connect, not ServerConnectedEvent: Velocity adds the player to the server's + // `playersConnected` after that event, and releasing first would let the next pick see the + // lobby one player short. + @Subscribe + fun onServerPostConnect(event: ServerPostConnectEvent) { + release(event.player.uniqueId) + } + + @Subscribe + fun onDisconnect(event: DisconnectEvent) { + release(event.player.uniqueId) + } + } + + companion object { + /** Long enough for a slow join, short enough that a lost one frees its place quickly. */ + val DEFAULT_TTL: Duration = Duration.ofSeconds(15) + } +} diff --git a/velocity/src/main/kotlin/gg/grounds/discovery/LobbySelection.kt b/velocity/src/main/kotlin/gg/grounds/discovery/LobbySelection.kt index 0bc9ce6..50174c4 100644 --- a/velocity/src/main/kotlin/gg/grounds/discovery/LobbySelection.kt +++ b/velocity/src/main/kotlin/gg/grounds/discovery/LobbySelection.kt @@ -3,20 +3,17 @@ package gg.grounds.discovery /** * Which lobby a joining player should land on. * - * Least-occupied wins. Joins spread across the lobbies instead of filling one and then the next, so - * no single instance holds the whole region: a lobby restart takes a share of the players with it - * rather than all of them. + * Fullest first, below a per-lobby cap. A network of 50 players then feels like one lobby of 50 + * rather than fifty lobbies of one, and only when every lobby is at the cap does a join go to an + * emptier one. The fleet keeps one empty lobby Ready on top of the allocated ones, so "an emptier + * one" is normally that spare: it takes the overflow, fills, and the autoscaler starts the next. * - * This also gives a newly autoscaled lobby what it needs without any special case. A fresh lobby is - * empty, so it is the least occupied, so it takes joins until it has caught up with the others — - * priority filling falls out of the same rule that does the spreading. + * This replaces least-occupied-first. Together with that Ready buffer it put every player in a + * lobby of their own: the spare was always the least occupied, the first join allocated it, the + * autoscaler started another empty one, and the next join took that. * - * This replaces a fullest-first-below-a-cap policy. That one existed so a network of 50 would feel - * like one lobby of 50 rather than five of ten, which is a real concern — but it meant that in - * practice every player in a region sat in one process, and measured on stage the lobby was never - * the reason to split: 228 players cost 0.29 cores and a 2 ms average tick against a 50 ms budget. - * The reason to split is blast radius, and that argues for spreading always rather than for a - * threshold nobody ever reached. + * When every lobby is at the cap the least occupied one takes the join, so the overflow spreads + * instead of piling onto one lobby while the spare is still starting. * * Ties break on the name so that every proxy, working from the same counts, makes the same choice. * @@ -27,12 +24,19 @@ package gg.grounds.discovery */ object LobbySelection { + const val DEFAULT_PLAYER_CAP = 100 + data class Candidate(val name: String, val players: Int, val outdated: Boolean = false) - fun pick(candidates: List): String? = - candidates - .filter { !it.outdated } - .ifEmpty { candidates } - .minWithOrNull(compareBy({ it.players }, { it.name })) - ?.name + fun pick(candidates: List, cap: Int = DEFAULT_PLAYER_CAP): String? { + val pool = candidates.filter { !it.outdated }.ifEmpty { candidates } + val withRoom = pool.filter { it.players < cap } + val chosen = + if (withRoom.isNotEmpty()) { + withRoom.minWithOrNull(compareBy({ -it.players }, { it.name })) + } else { + pool.minWithOrNull(compareBy({ it.players }, { it.name })) + } + return chosen?.name + } } diff --git a/velocity/src/main/kotlin/gg/grounds/discovery/OutdatedLobbyMover.kt b/velocity/src/main/kotlin/gg/grounds/discovery/OutdatedLobbyMover.kt index f86533d..38c1616 100644 --- a/velocity/src/main/kotlin/gg/grounds/discovery/OutdatedLobbyMover.kt +++ b/velocity/src/main/kotlin/gg/grounds/discovery/OutdatedLobbyMover.kt @@ -15,13 +15,15 @@ import org.slf4j.Logger * Which players to move off outdated lobbies this tick, and where to. * * Only players standing on an outdated lobby move, at most [limit] of them, and only when a current - * lobby exists to take them. Each move counts towards its target, so one tick spreads its players - * the way [LobbySelection.pick] spreads joins instead of sending all of them to the same lobby. + * lobby exists to take them. Each move counts towards its target, so one tick fills the current + * lobbies the way [LobbySelection.pick] fills them with joins: the fullest one up to [cap], then + * the next. */ internal fun

planOutdatedLobbyMoves( players: List>, lobbies: List, limit: Int, + cap: Int = LobbySelection.DEFAULT_PLAYER_CAP, ): List> { val outdated = lobbies.filter { it.outdated }.mapTo(mutableSetOf()) { it.name } val counts = @@ -32,7 +34,10 @@ internal fun

planOutdatedLobbyMoves( .take(limit) .map { (player, _) -> val target = - LobbySelection.pick(counts.map { LobbySelection.Candidate(it.key, it.value) })!! + LobbySelection.pick( + counts.map { LobbySelection.Candidate(it.key, it.value) }, + cap, + )!! counts[target] = counts.getValue(target) + 1 player to target } @@ -56,6 +61,7 @@ class OutdatedLobbyMover( /** A draining proxy sends its lobby players to another proxy instead; moving them is moot. */ private val isDraining: () -> Boolean, private val maxMovesPerTick: Int = DEFAULT_MAX_MOVES_PER_TICK, + private val lobbyPlayerCap: Int = LobbySelection.DEFAULT_PLAYER_CAP, private val messages: Translations = Translations.forBundleWithSystemRenderer( "gg.grounds.agones.messages", @@ -93,7 +99,8 @@ class OutdatedLobbyMover( .mapNotNull { player -> player.currentServer.orElse(null)?.let { player to it.serverInfo.name } } - for ((player, target) in planOutdatedLobbyMoves(players, lobbies, maxMovesPerTick)) { + for ((player, target) in + planOutdatedLobbyMoves(players, lobbies, maxMovesPerTick, lobbyPlayerCap)) { move(player, target) } } diff --git a/velocity/src/test/kotlin/gg/grounds/discovery/DiscoveryConfigTest.kt b/velocity/src/test/kotlin/gg/grounds/discovery/DiscoveryConfigTest.kt index fa2f54f..5bf2ac8 100644 --- a/velocity/src/test/kotlin/gg/grounds/discovery/DiscoveryConfigTest.kt +++ b/velocity/src/test/kotlin/gg/grounds/discovery/DiscoveryConfigTest.kt @@ -20,6 +20,23 @@ class DiscoveryConfigTest { assertEquals("PodIP", cfg.addressType) assertEquals(25565, cfg.port) assertEquals(emptyList(), cfg.staticServers) + assertEquals(100, cfg.lobbyPlayerCap) + } + + @Test + fun `GROUNDS_LOBBY_PLAYER_CAP sets the cap and a nonsense value keeps the default`() { + assertEquals( + 40, + DiscoveryConfig.fromEnv(mapOf("GROUNDS_LOBBY_PLAYER_CAP" to " 40 ")).lobbyPlayerCap, + ) + assertEquals( + 100, + DiscoveryConfig.fromEnv(mapOf("GROUNDS_LOBBY_PLAYER_CAP" to "0")).lobbyPlayerCap, + ) + assertEquals( + 100, + DiscoveryConfig.fromEnv(mapOf("GROUNDS_LOBBY_PLAYER_CAP" to "lots")).lobbyPlayerCap, + ) } @Test diff --git a/velocity/src/test/kotlin/gg/grounds/discovery/DiscoveryServiceTest.kt b/velocity/src/test/kotlin/gg/grounds/discovery/DiscoveryServiceTest.kt index e1b5e60..d87e12c 100644 --- a/velocity/src/test/kotlin/gg/grounds/discovery/DiscoveryServiceTest.kt +++ b/velocity/src/test/kotlin/gg/grounds/discovery/DiscoveryServiceTest.kt @@ -132,7 +132,7 @@ class DiscoveryServiceTest { service.refreshRetiredGameServerSets { throw IllegalStateException("403 Forbidden") } assertFalse(service.isOutdated("lobby-old")) - // Back to plain least-occupied: the empty old lobby wins again. + // Back to plain fullest-first: the old lobby, holding the player, wins again. assertEquals("lobby-old", service.pickLobby()?.serverInfo?.name) } @@ -158,8 +158,8 @@ class DiscoveryServiceTest { } /** - * Two registered lobbies, the old one empty and the new one holding a player — so plain - * least-occupied would pick the old one, and only the outdated check sends joins elsewhere. + * Two registered lobbies, the old one holding a player and the new one empty — so plain + * fullest-first would pick the old one, and only the outdated check sends joins elsewhere. */ private fun lobbyService(): DiscoveryService { val oldLobby = @@ -170,7 +170,7 @@ class DiscoveryServiceTest { "lobby-old", InetSocketAddress.createUnresolved("10.0.0.1", 25565), ), - "getPlayersConnected" to emptyList(), + "getPlayersConnected" to listOf(proxy()), ) ) val newLobby = @@ -181,7 +181,7 @@ class DiscoveryServiceTest { "lobby-new", InetSocketAddress.createUnresolved("10.0.0.2", 25565), ), - "getPlayersConnected" to listOf(proxy()), + "getPlayersConnected" to emptyList(), ) ) val servers = listOf(oldLobby, newLobby) diff --git a/velocity/src/test/kotlin/gg/grounds/discovery/LobbyKickListenerTest.kt b/velocity/src/test/kotlin/gg/grounds/discovery/LobbyKickListenerTest.kt index a796d00..84b0978 100644 --- a/velocity/src/test/kotlin/gg/grounds/discovery/LobbyKickListenerTest.kt +++ b/velocity/src/test/kotlin/gg/grounds/discovery/LobbyKickListenerTest.kt @@ -6,6 +6,7 @@ import com.velocitypowered.api.proxy.server.RegisteredServer import com.velocitypowered.api.proxy.server.ServerInfo import java.lang.reflect.Proxy import java.net.InetSocketAddress +import java.util.UUID import net.kyori.adventure.text.Component import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertSame @@ -15,12 +16,15 @@ import org.junit.jupiter.api.Test class LobbyKickListenerTest { private val oldLobby = server("lobby-old") private val newLobby = server("lobby-new") + private val playerId = UUID.randomUUID() private val excluded = mutableListOf() + private val placedFor = mutableListOf() private val listener = LobbyKickListener( isLobby = { it.startsWith("lobby-") }, - pickLobby = { exclude -> + pickLobby = { exclude, player -> excluded.add(exclude) + placedFor.add(player) newLobby }, ) @@ -34,13 +38,14 @@ class LobbyKickListenerTest { val result = event.result as KickedFromServerEvent.RedirectPlayer assertSame(newLobby, result.server) assertEquals(listOf("lobby-old"), excluded) + assertEquals(listOf(playerId), placedFor) } @Test fun `no other lobby leaves Velocity's disconnect in place`() { val event = kick(from = oldLobby) - LobbyKickListener({ true }, { null }).onKickedFromServer(event) + LobbyKickListener({ true }, { _, _ -> null }).onKickedFromServer(event) assertTrue(event.result is KickedFromServerEvent.DisconnectPlayer) } @@ -81,7 +86,7 @@ class LobbyKickListenerTest { KickedFromServerEvent.DisconnectPlayer.create(Component.text("Server closed")), ) = KickedFromServerEvent( - proxy(mapOf("isActive" to active)), + proxy(mapOf("isActive" to active, "getUniqueId" to playerId)), from, Component.text("Server closed"), false, diff --git a/velocity/src/test/kotlin/gg/grounds/discovery/LobbyPlacementTest.kt b/velocity/src/test/kotlin/gg/grounds/discovery/LobbyPlacementTest.kt new file mode 100644 index 0000000..85e7e82 --- /dev/null +++ b/velocity/src/test/kotlin/gg/grounds/discovery/LobbyPlacementTest.kt @@ -0,0 +1,110 @@ +package gg.grounds.discovery + +import gg.grounds.discovery.LobbySelection.Candidate +import java.time.Duration +import java.util.UUID +import java.util.concurrent.Callable +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +class LobbyPlacementTest { + + @Test + fun `concurrent joins never put more than the cap on one lobby`() { + val placement = LobbyPlacement(cap = 100) + // The observed counts do not move: none of these players has arrived yet, so only the + // reservations stand between the burst and one lobby of 150. + val observed = listOf(Candidate("lobby-a", 0), Candidate("lobby-b", 0)) + val pool = Executors.newFixedThreadPool(16) + val start = CountDownLatch(1) + try { + val picks = + (1..150).map { + pool.submit( + Callable { + start.await() + placement.place(observed, UUID.randomUUID()) + } + ) + } + start.countDown() + val counts = picks.map { it.get(10, TimeUnit.SECONDS) }.groupingBy { it }.eachCount() + + assertEquals(mapOf("lobby-a" to 100, "lobby-b" to 50), counts) + } finally { + pool.shutdownNow() + } + } + + @Test + fun `reservations top up a lobby that already has players`() { + val placement = LobbyPlacement(cap = 100) + val observed = listOf(Candidate("lobby-a", 97), Candidate("lobby-b", 0)) + + val picks = (1..5).map { placement.place(observed, UUID.randomUUID()) } + + assertEquals(List(3) { "lobby-a" } + List(2) { "lobby-b" }, picks) + } + + @Test + fun `a reservation counts until the player arrives`() { + val placement = LobbyPlacement(cap = 100) + val player = UUID.randomUUID() + val observed = listOf(Candidate("lobby-a", 10)) + + placement.place(observed, player) + assertEquals(11, placement.withReservations(observed).single().players) + + // Arrived: Velocity now counts them, so the reservation must not count them twice. + placement.release(player) + assertEquals(10, placement.withReservations(observed).single().players) + } + + @Test + fun `a reservation whose join never completes expires`() { + var now = 0L + val placement = LobbyPlacement(cap = 100, ttl = Duration.ofSeconds(15), clock = { now }) + val observed = listOf(Candidate("lobby-a", 10)) + + placement.place(observed, UUID.randomUUID()) + now += Duration.ofSeconds(16).toNanos() + + assertEquals(10, placement.withReservations(observed).single().players) + } + + @Test + fun `the excluded lobby is never chosen even when it is the fullest`() { + val placement = LobbyPlacement(cap = 100) + val observed = listOf(Candidate("lobby-a", 50), Candidate("lobby-b", 10)) + + assertEquals("lobby-b", placement.place(observed, UUID.randomUUID(), exclude = "LOBBY-A")) + } + + @Test + fun `a failed join placed again moves its reservation instead of keeping both`() { + val placement = LobbyPlacement(cap = 100) + val player = UUID.randomUUID() + val observed = listOf(Candidate("lobby-a", 50), Candidate("lobby-b", 10)) + + assertEquals("lobby-a", placement.place(observed, player)) + // lobby-a refused the connection; the kick listener places the player elsewhere. + assertEquals("lobby-b", placement.place(observed, player, exclude = "lobby-a")) + + assertEquals( + mapOf("lobby-a" to 50, "lobby-b" to 11), + placement.withReservations(observed).associate { it.name to it.players }, + ) + } + + @Test + fun `asking without a player reserves nothing`() { + val placement = LobbyPlacement(cap = 100) + val observed = listOf(Candidate("lobby-a", 10)) + + assertEquals("lobby-a", placement.place(observed)) + assertEquals(10, placement.withReservations(observed).single().players) + } +} diff --git a/velocity/src/test/kotlin/gg/grounds/discovery/LobbySelectionTest.kt b/velocity/src/test/kotlin/gg/grounds/discovery/LobbySelectionTest.kt index 5d8362e..988aaa8 100644 --- a/velocity/src/test/kotlin/gg/grounds/discovery/LobbySelectionTest.kt +++ b/velocity/src/test/kotlin/gg/grounds/discovery/LobbySelectionTest.kt @@ -8,41 +8,76 @@ import org.junit.jupiter.api.Test class LobbySelectionTest { @Test - fun `joins go to the least occupied lobby`() { + fun `joins go to the fullest lobby below the cap`() { val chosen = LobbySelection.pick( - listOf(Candidate("lobby-a", 12), Candidate("lobby-b", 391), Candidate("lobby-c", 3)) + listOf(Candidate("lobby-a", 12), Candidate("lobby-b", 99), Candidate("lobby-c", 3)) ) - assertEquals("lobby-c", chosen) + assertEquals("lobby-b", chosen) + } + + @Test + fun `a lobby at the cap takes no more joins`() { + val chosen = + LobbySelection.pick(listOf(Candidate("lobby-a", 100), Candidate("lobby-b", 40))) + assertEquals("lobby-b", chosen) + } + + @Test + fun `the empty spare only takes joins once every other lobby is full`() { + val partly = + LobbySelection.pick(listOf(Candidate("lobby-a", 1), Candidate("lobby-spare", 0))) + val full = + LobbySelection.pick(listOf(Candidate("lobby-a", 100), Candidate("lobby-spare", 0))) + + assertEquals("lobby-a", partly) + assertEquals("lobby-spare", full) } @Test - fun `a freshly autoscaled lobby is filled first because it is empty`() { + fun `when every lobby is full the least occupied takes the overflow`() { val chosen = LobbySelection.pick( listOf( - Candidate("lobby-a", 150), - Candidate("lobby-b", 148), - Candidate("lobby-new", 0), + Candidate("lobby-a", 103), + Candidate("lobby-b", 100), + Candidate("lobby-c", 101), ) ) - assertEquals("lobby-new", chosen) + assertEquals("lobby-b", chosen) } @Test - fun `repeated picks even the lobbies out rather than filling one`() { - val counts = mutableMapOf("lobby-a" to 4, "lobby-b" to 0, "lobby-c" to 2) - repeat(6) { - val chosen = LobbySelection.pick(counts.map { Candidate(it.key, it.value) })!! - counts[chosen] = counts.getValue(chosen) + 1 - } - assertEquals(listOf(4, 4, 4), counts.values.sorted()) + fun `repeated picks fill one lobby before the next`() { + val counts = linkedMapOf("lobby-a" to 0, "lobby-b" to 0) + val picks = + (1..6).map { + val chosen = + LobbySelection.pick(counts.map { Candidate(it.key, it.value) }, cap = 3)!! + counts[chosen] = counts.getValue(chosen) + 1 + chosen + } + assertEquals(List(3) { "lobby-a" } + List(3) { "lobby-b" }, picks) } @Test fun `ties break on the name so every proxy makes the same choice`() { - val chosen = LobbySelection.pick(listOf(Candidate("lobby-b", 50), Candidate("lobby-a", 50))) - assertEquals("lobby-a", chosen) + assertEquals( + "lobby-a", + LobbySelection.pick(listOf(Candidate("lobby-b", 50), Candidate("lobby-a", 50))), + ) + assertEquals( + "lobby-a", + LobbySelection.pick(listOf(Candidate("lobby-b", 100), Candidate("lobby-a", 100))), + ) + } + + @Test + fun `the cap is configurable`() { + val candidates = listOf(Candidate("lobby-a", 20), Candidate("lobby-b", 5)) + + assertEquals("lobby-a", LobbySelection.pick(candidates, cap = 21)) + assertEquals("lobby-b", LobbySelection.pick(candidates, cap = 20)) } @Test @@ -54,7 +89,7 @@ class LobbySelectionTest { fun `an outdated lobby takes no joins while a current one exists`() { val chosen = LobbySelection.pick( - listOf(Candidate("lobby-old", 0, outdated = true), Candidate("lobby-new", 40)) + listOf(Candidate("lobby-old", 60, outdated = true), Candidate("lobby-new", 4)) ) assertEquals("lobby-new", chosen) } @@ -68,6 +103,6 @@ class LobbySelectionTest { Candidate("lobby-b", 2, outdated = true), ) ) - assertEquals("lobby-b", chosen) + assertEquals("lobby-a", chosen) } } diff --git a/velocity/src/test/kotlin/gg/grounds/discovery/OutdatedLobbyMoverTest.kt b/velocity/src/test/kotlin/gg/grounds/discovery/OutdatedLobbyMoverTest.kt index 20b9308..e6370a7 100644 --- a/velocity/src/test/kotlin/gg/grounds/discovery/OutdatedLobbyMoverTest.kt +++ b/velocity/src/test/kotlin/gg/grounds/discovery/OutdatedLobbyMoverTest.kt @@ -60,7 +60,7 @@ class OutdatedLobbyMoverTest { } @Test - fun `one tick spreads its moves across the current lobbies`() { + fun `one tick fills the fullest current lobby up to the cap, then the next`() { val moves = planOutdatedLobbyMoves( (1..6).map { "p$it" to "lobby-old" }, @@ -70,9 +70,10 @@ class OutdatedLobbyMoverTest { Candidate("lobby-b", 0), ), limit = 10, + cap = 5, ) - // 3+0 -> b, b, b, then a and b alternate: both end within one player of each other. + // lobby-a is the fuller one: it takes two and is at the cap, lobby-b takes the rest. assertEquals( mapOf("lobby-a" to 2, "lobby-b" to 4), moves.groupingBy { it.second }.eachCount(), From 218bc86a8d62aee39cb620452e45a78d1141b616 Mon Sep 17 00:00:00 2001 From: Hendrik Brombeer <20863417+hbrombeer@users.noreply.github.com> Date: Sat, 17 Oct 2026 17:54:22 +0200 Subject: [PATCH 2/2] feat(velocity): prefer the home lobby while it has room pickLobby(UUID, String?, String?) takes the lobby a player queued from as a preference: it wins while it is a current lobby below the cap, counting this proxy's reservations, and the fill rule decides otherwise. plugin-match passes it for returns after a match. --- README.md | 4 +- .../kotlin/gg/grounds/GroundsPluginAgones.kt | 8 ++-- .../gg/grounds/discovery/DiscoveryService.kt | 11 +++-- .../gg/grounds/discovery/LobbyPlacement.kt | 15 ++++++- .../grounds/discovery/LobbyPlacementTest.kt | 45 +++++++++++++++++++ 5 files changed, 75 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index fc40b65..ea8ac38 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,9 @@ This only registers the backend with Velocity; configuring the backend itself fo forwarding remains a separate deployment concern. Joins, lobby kicks and plugin-match's returns from a match all go to the fullest lobby that is -below `GROUNDS_LOBBY_PLAYER_CAP`, and to an emptier one only when every lobby is at the cap. +below `GROUNDS_LOBBY_PLAYER_CAP`, and to an emptier one only when every lobby is at the cap. A +return goes back to the lobby the player queued from first, while that lobby is current and below +the cap. Counts are the larger of the network-wide count (plugin-proxy) and this proxy's own, plus the players this proxy has sent but who have not arrived yet, so a burst of joins on one proxy cannot overfill a lobby. Across proxies the cap is soft by the couple of seconds the network count lags. diff --git a/velocity/src/main/kotlin/gg/grounds/GroundsPluginAgones.kt b/velocity/src/main/kotlin/gg/grounds/GroundsPluginAgones.kt index 7ff1d2c..dcc1af8 100644 --- a/velocity/src/main/kotlin/gg/grounds/GroundsPluginAgones.kt +++ b/velocity/src/main/kotlin/gg/grounds/GroundsPluginAgones.kt @@ -161,15 +161,17 @@ constructor(private val proxyServer: ProxyServer, private val logger: Logger) { /** * The lobby [playerId] should go to, never [exclude], with their place reserved; null when - * there is none or discovery has not started. + * there is none or discovery has not started. [preferred] — the lobby they queued from — is + * taken while it is current and below the cap, reservations included; otherwise the fill rule + * decides. * * For plugin-match, which sends players back to a lobby after a match. It calls this by * reflection — JDK types only — so the two plugins share one placement rule and one set of * reservations without a compile-time dependency on each other. */ - fun pickLobby(playerId: UUID, exclude: String?): String? = + fun pickLobby(playerId: UUID, exclude: String?, preferred: String?): String? = if (this::discoveryService.isInitialized) { - discoveryService.pickLobby(exclude, playerId)?.serverInfo?.name + discoveryService.pickLobby(exclude, playerId, preferred)?.serverInfo?.name } else { null } diff --git a/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryService.kt b/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryService.kt index 7e31887..ccdb579 100644 --- a/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryService.kt +++ b/velocity/src/main/kotlin/gg/grounds/discovery/DiscoveryService.kt @@ -120,10 +120,15 @@ class DiscoveryService( /** * The lobby [playerId] should be sent to, never [exclude]; see [LobbySelection.pick]. With a - * [playerId] the place is reserved until they arrive; without one this only asks. + * [playerId] the place is reserved until they arrive; without one this only asks. [preferred] + * is taken while it has room; see [LobbyPlacement.place]. */ - fun pickLobby(exclude: String? = null, playerId: UUID? = null): RegisteredServer? { - val chosen = placement.place(observedLobbies(), playerId, exclude) ?: return null + fun pickLobby( + exclude: String? = null, + playerId: UUID? = null, + preferred: String? = null, + ): RegisteredServer? { + val chosen = placement.place(observedLobbies(), playerId, exclude, preferred) ?: return null return proxyServer.getServer(chosen).orElse(null) } diff --git a/velocity/src/main/kotlin/gg/grounds/discovery/LobbyPlacement.kt b/velocity/src/main/kotlin/gg/grounds/discovery/LobbyPlacement.kt index 9f3a9c0..6859e9d 100644 --- a/velocity/src/main/kotlin/gg/grounds/discovery/LobbyPlacement.kt +++ b/velocity/src/main/kotlin/gg/grounds/discovery/LobbyPlacement.kt @@ -37,12 +37,17 @@ class LobbyPlacement( * The lobby for [playerId], never [exclude]; null when there is none. [observed] are the * lobbies with the players already counted on them. A null [playerId] only asks — nothing is * reserved. + * + * [preferred] wins over the fill rule while it is a current lobby below the cap, reservations + * included — the lobby a player queued from, for a return after their match. Gone, outdated or + * full, it is ignored and the fill rule decides. */ @Synchronized fun place( observed: List, playerId: UUID? = null, exclude: String? = null, + preferred: String? = null, ): String? { // A player placed again (kicked before arriving, say) must not count against themselves. if (playerId != null) reservations.remove(playerId) @@ -50,7 +55,15 @@ class LobbyPlacement( withReservations(observed).filter { exclude == null || canonicalServerName(it.name) != canonicalServerName(exclude) } - val chosen = LobbySelection.pick(candidates, cap) ?: return null + val home = + preferred?.let { name -> + candidates.firstOrNull { + canonicalServerName(it.name) == canonicalServerName(name) && + !it.outdated && + it.players < cap + } + } + val chosen = home?.name ?: LobbySelection.pick(candidates, cap) ?: return null if (playerId != null) { reservations[playerId] = Reservation(canonicalServerName(chosen), clock() + ttl.toNanos()) diff --git a/velocity/src/test/kotlin/gg/grounds/discovery/LobbyPlacementTest.kt b/velocity/src/test/kotlin/gg/grounds/discovery/LobbyPlacementTest.kt index 85e7e82..30289bf 100644 --- a/velocity/src/test/kotlin/gg/grounds/discovery/LobbyPlacementTest.kt +++ b/velocity/src/test/kotlin/gg/grounds/discovery/LobbyPlacementTest.kt @@ -107,4 +107,49 @@ class LobbyPlacementTest { assertEquals("lobby-a", placement.place(observed)) assertEquals(10, placement.withReservations(observed).single().players) } + + @Test + fun `the preferred lobby wins while it has room`() { + val placement = LobbyPlacement(cap = 100) + val observed = listOf(Candidate("lobby-home", 10), Candidate("lobby-full", 90)) + + assertEquals( + "lobby-home", + placement.place(observed, UUID.randomUUID(), preferred = "lobby-home"), + ) + assertEquals(11, placement.withReservations(observed).first().players) + } + + @Test + fun `a preferred lobby at the cap counting reservations falls back to the fill rule`() { + val placement = LobbyPlacement(cap = 100) + val observed = listOf(Candidate("lobby-home", 99), Candidate("lobby-b", 40)) + + assertEquals( + "lobby-home", + placement.place(observed, UUID.randomUUID(), preferred = "lobby-home"), + ) + // The reservation above fills it: the next returning player goes to the fullest with room. + assertEquals( + "lobby-b", + placement.place(observed, UUID.randomUUID(), preferred = "lobby-home"), + ) + } + + @Test + fun `a preferred lobby that is gone, outdated or excluded is ignored`() { + val placement = LobbyPlacement(cap = 100) + val observed = listOf(Candidate("lobby-old", 5, outdated = true), Candidate("lobby-b", 40)) + + assertEquals("lobby-b", placement.place(observed, preferred = "lobby-gone")) + assertEquals("lobby-b", placement.place(observed, preferred = "lobby-old")) + assertEquals( + "lobby-b", + placement.place( + listOf(Candidate("lobby-home", 5), Candidate("lobby-b", 40)), + exclude = "lobby-home", + preferred = "lobby-home", + ), + ) + } }