yuris/spacetimedb-defold

Fork 0

reconnect: a mass cold start still retries in lockstep #65

New issue

Closed

opened 2026-08-20 14:10:41 +00:00 by yuris · 1 comment

yuris commented

2026-08-20 14:10:41 +00:00

Owner

Found by review of #62 (closes #19), and out of scope for it.

#19 spreads the reconnect backoff per client, keyed on the identity. But the spread comes from conn.identity, which is nil until a socket succeeds — so the one case it does not cover is the synchronised cold start: the server goes down, the whole player base relaunches, and every client retries at the flat 1, 2, 4, 8, 16, 30.

Measured on the #19 branch:

storage.load() returns { identity = "0xc200deadbeef", token = "tok" }
conn.identity after connection.new = nil
attempts 1..6 -> 1, 2, 4, 8, 16, 30

The data needed is already loaded three lines above the callsite. connect reads persisted.token from storage but never persisted.identity, even though the identity IS written to storage (dev/test/test_connection.lua asserts saved.identity), and connection.new initialises identity = nil.

So the fix looks like a one-liner: read the persisted identity and pass it to spread_for for the pre-connection attempts. The reason it was not folded into #19 is that it is a real judgement call rather than an oversight — the on-disk identity is not server-confirmed at that point, so this means spreading on an unverified value. Cheap, but it deserves deciding on its own terms.

Worth noting this is arguably the MORE important herd: a server restart that drops live sockets is covered by #19, but a server outage that makes everyone relaunch is not, and it is the one that coincides with the server being least able to absorb it.

Found by review of #62 (closes #19), and out of scope for it. #19 spreads the reconnect backoff per client, keyed on the identity. But the spread comes from conn.identity, which is nil until a socket succeeds — so the one case it does not cover is the synchronised cold start: the server goes down, the whole player base relaunches, and every client retries at the flat 1, 2, 4, 8, 16, 30. Measured on the #19 branch: storage.load() returns { identity = "0xc200deadbeef", token = "tok" } conn.identity after connection.new = nil attempts 1..6 -> 1, 2, 4, 8, 16, 30 The data needed is already loaded three lines above the callsite. connect reads persisted.token from storage but never persisted.identity, even though the identity IS written to storage (dev/test/test_connection.lua asserts saved.identity), and connection.new initialises identity = nil. So the fix looks like a one-liner: read the persisted identity and pass it to spread_for for the pre-connection attempts. The reason it was not folded into #19 is that it is a real judgement call rather than an oversight — the on-disk identity is not server-confirmed at that point, so this means spreading on an unverified value. Cheap, but it deserves deciding on its own terms. Worth noting this is arguably the MORE important herd: a server restart that drops live sockets is covered by #19, but a server outage that makes everyone relaunch is not, and it is the one that coincides with the server being least able to absorb it.

yuris referenced this issue

2026-08-20 14:15:21 +00:00

v1.1.0 release checklist #59

yuris referenced this issue

2026-08-20 16:01:40 +00:00

v1.1.0 release checklist #59

yuris referenced this issue from a commit

2026-08-20 16:22:59 +00:00

review: correct ten README claims the truth check measured as wrong

yuris referenced this issue

2026-08-20 16:31:55 +00:00

v1.1.0 release checklist #59

yuris referenced this issue

2026-08-20 16:32:45 +00:00

v1.1.0 release checklist #59

yuris referenced this issue

2026-08-20 16:41:16 +00:00

v1.1.0 release checklist #59

yuris referenced this issue from a commit

2026-08-20 17:03:55 +00:00

feat: seed the reconnect spread from the persisted identity (#65)

yuris referenced this issue from a pull request that will close it,

2026-08-20 17:04:54 +00:00

feat: seed the reconnect spread from the persisted identity #72

yuris referenced this issue from a commit

2026-08-20 17:13:53 +00:00

fix: type-check the persisted identity before it reaches backoff_spread (#65)

yuris referenced this issue

2026-08-20 17:29:32 +00:00

feat: seed the reconnect spread from the persisted identity #72

yuris closed this issue

2026-08-20 17:30:15 +00:00

yuris commented

2026-08-20 18:48:50 +00:00

Author

Owner

Closed by #72.

connect now seeds the reconnect spread from the persisted identity when there is one, so a fleet cold-starting into an outage scatters instead of sharing the flat 1/2/4/8/16/30 ladder. persisted.identity was already being written by connection.lua's IdentityToken handler and never read back; it is now read for this one purpose and confined to the backoff_spread argument.

On the judgement call this issue named: the identity is used although it is not server-confirmed at that point. A wrong identity gives a wrong-but-deterministic slot, which is strictly better than every client sharing one ladder — the spread only needs a value that differs between clients, not a correct one. The wire identity takes over the moment IdentityToken lands, via conn.identity or persisted_identity. A genuinely fresh install has nothing persisted and keeps the exact ladder, which is right: one new client is not a herd.

One real bug came out of review. backoff_spread does #identity then identity:byte(i), and the storage slot is shared (see sys_storage), so a caller's own identity key or a hand-edited save could put a number or a table there — turning every socket drop into a raise from inside schedule_reconnect, i.e. out of the engine's websocket callback:

./stdb/connection.lua:681: attempt to get length of local 'identity' (a number value)

Now type-checked at the read, following auth.lua's guard on the same seam; a non-string degrades to the exact ladder. Tests cover the number and table cases and fail without the check.

Also corrected two now-false statements this change created: sys_storage's comment claiming connect reads only persisted.token, and backoff_spread's docstring clause "the first connect is a cold start, not a herd" — which was the policy this issue exists to overturn.

Verified on main @ 52dd72d: 8 passed, 0 failed, stylua --check clean, native and HTML5 conformance pass. Reconnect prose and the connection.backoff_spread API row in the README updated.

Closed by #72. `connect` now seeds the reconnect spread from the persisted identity when there is one, so a fleet cold-starting into an outage scatters instead of sharing the flat 1/2/4/8/16/30 ladder. `persisted.identity` was already being written by `connection.lua`'s `IdentityToken` handler and never read back; it is now read for this one purpose and confined to the `backoff_spread` argument. **On the judgement call this issue named:** the identity is used although it is not server-confirmed at that point. A wrong identity gives a wrong-but-deterministic slot, which is strictly better than every client sharing one ladder — the spread only needs a value that differs between clients, not a correct one. The wire identity takes over the moment `IdentityToken` lands, via `conn.identity or persisted_identity`. A genuinely fresh install has nothing persisted and keeps the exact ladder, which is right: one new client is not a herd. One real bug came out of review. `backoff_spread` does `#identity` then `identity:byte(i)`, and the storage slot is shared (see `sys_storage`), so a caller's own `identity` key or a hand-edited save could put a number or a table there — turning every socket drop into a raise from inside `schedule_reconnect`, i.e. out of the engine's websocket callback: ``` ./stdb/connection.lua:681: attempt to get length of local 'identity' (a number value) ``` Now type-checked at the read, following `auth.lua`'s guard on the same seam; a non-string degrades to the exact ladder. Tests cover the number and table cases and fail without the check. Also corrected two now-false statements this change created: `sys_storage`'s comment claiming `connect` reads only `persisted.token`, and `backoff_spread`'s docstring clause "the first connect is a cold start, not a herd" — which was the policy this issue exists to overturn. Verified on `main` @ 52dd72d: `8 passed, 0 failed`, `stylua --check` clean, native and HTML5 conformance pass. Reconnect prose and the `connection.backoff_spread` API row in the README updated.