fix: detect Server_id collisions between live servers - #184
Open
EoD wants to merge 2 commits into
Open
Conversation
Unify the e2e scenarios by housting them into bot/lib.js as summarizeInventory and dbInventory, plus rconAsk and setDbLatency on the harness. This drops multiple lines of duplicated code and leaves each scenario stating only what it is about. lib.js also gains the helpers for driving the stack itself - run, compose, composeFile, containerId, logCount, dataVolume and loadedModConfig - so a scenario that has to recreate a server under a different config starts from shared code rather than from a fresh copy of `docker compose` plumbing. Both servers also shared one toxiproxy proxy, so a latency toxic slowed both of them at once and could never single out one server's database traffic - which is exactly what a scenario about one server's writes racing the other's needs. Give server-b its own proxy on 3307 (config/server-b now points at it), and make setDbLatency take the server whose traffic it slows, so a caller cannot forget which one it meant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every ownership check in the mod keys on Server_id: markOffline clears online only while last_server is still us, and the already-online kick looks up the server owning a session. Two nodes sharing an id turn all of them into silent no-ops, and the usual way to stand up a second node, copying the first one's config, produces exactly that. The random default only protects a config generated from scratch, and nothing surfaced the state: the boot claim was a blind upsert, and no log printed the id the server had resolved. Stamp a random non-zero token into a new server_info.boot_token at boot. The heartbeat that already refreshed last_update now refreshes it only while the row still carries our token, or the zero of a row written before the column existed. A refresh that matches nothing means another live server has stamped the row: report it, then take the row back, so both nodes keep reporting for as long as the collision lasts. Boot also logs the resolved id and flags a row that still looks live. Both only log. A clean shutdown leaves the row inside the 300s liveness window, so refusing to start would lock out crash recovery and quick restarts, and the heartbeat catches a real collision within seconds. Adding the column tolerates the duplicate-column error at this call site, since two nodes booting against a fresh database can both try. test-server-id-collision.js recreates server-b with server-a's id and requires both servers to report the collision, keep reporting it, and fall silent once the ids are distinct again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
EoD
force-pushed
the
detect-server-id-collisions
branch
from
August 21, 2026 19:37
9bf4c72 to
c77afee
Compare
Collaborator
Author
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.
@mlus-asuka This is a rather fundamental change how servers IDs are handled. If you have any questions, let me know.
Server_id is the identity every ownership check in the mod is written against: markOffline's "online=0 only if last_server is still us" compare-and-set, the already-online kick's lookup of the server owning a session, the boot-time repair of stale online flags. Two servers sharing one id turn all of them into silent no-ops, and the normal way to stand up a second node - copying the first node's config - produces exactly that. The random default id only protects a config generated from scratch, and nothing surfaced the state either: the boot claim was a blind INSERT ... ON DUPLICATE KEY UPDATE, and no log printed the id the server had resolved.
Give server_info a boot_token column and stamp a random non-zero token into the row at boot. The heartbeat that already refreshed last_update from the tick loop now refreshes it only while the row still carries our token, or the zero of a row written before the column existed. A refresh that matches nothing means another running server has stamped the row; report that, then take the row back, so both servers keep reporting for as long as the collision lasts. The refresh stays one statement because it runs on the server thread, where each round-trip opens its own connection - naming the other server costs two more, paid only by a fleet already in that state.
Adding the column has to survive the race addColumnIfNotExists carries: it checks INFORMATION_SCHEMA and ALTERs in two statements, so two nodes booting against one fresh database can both decide to add it and the loser dies mid-boot. Tolerate the duplicate-column error at this call site alone, where the column exists either way, rather than paper over the helper's general race.
The boot claim also logs the id it resolved and flags an id whose row still looks like a live server's. Both only log: a clean shutdown leaves the row inside the 300s liveness window, so refusing to start would lock out every crash recovery and quick restart, while the heartbeat catches a real collision within seconds anyway.
e2e/bot/test-server-id-collision.js recreates server-b with server-a's Server_id through docker-compose.collision.yml, and requires both servers to report the collision, to keep reporting it, and to fall silent once the ids are distinct again. It runs last in the suite because it recreates a server mid-run.