diff --git a/README.md b/README.md index 8068ec0..ea8ac38 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,14 @@ 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. 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. + 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..dcc1af8 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,23 @@ 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. [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?, preferred: String?): String? = + if (this::discoveryService.isInitialized) { + discoveryService.pickLobby(exclude, playerId, preferred)?.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..ccdb579 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,34 @@ 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. [preferred] + * is taken while it has room; see [LobbyPlacement.place]. + */ + 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) + } + + /** + * 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 +146,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 +190,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..6859e9d --- /dev/null +++ b/velocity/src/main/kotlin/gg/grounds/discovery/LobbyPlacement.kt @@ -0,0 +1,110 @@ +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. + * + * [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) + val candidates = + withReservations(observed).filter { + exclude == null || canonicalServerName(it.name) != canonicalServerName(exclude) + } + 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()) + } + 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..30289bf --- /dev/null +++ b/velocity/src/test/kotlin/gg/grounds/discovery/LobbyPlacementTest.kt @@ -0,0 +1,155 @@ +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) + } + + @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", + ), + ) + } +} 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(),