yuris/spacetimedb-defold

Fork 0

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

Merged

yuris merged 2 commits from feat/65-cold-start-spread into main

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

yuris commented

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

Owner

Closes #65.

connect already spreads each reconnect delay by a deterministic 50–99.95% keyed on the identity, so a server restart does not bring the whole player base back in one instant. But backoff_spread was fed conn.identity, which is nil until a socket has succeeded — so the one case the spread exists for, a fleet relaunching into an outage, got no spread at all and retried on the flat 1/2/4/8/16/30 ladder in lockstep.

persisted.identity was already being written by connection.lua's IdentityToken handler and never read back. This reads it back, for this one purpose:

local persisted_identity = persisted and persisted.identity or nil
...
M.connection.backoff_spread(conn.identity or persisted_identity, attempt)

The judgement call

The persisted identity is not server-confirmed at that point — nothing has connected yet. Using it anyway is deliberate. The failure mode of a wrong identity is a wrong-but-deterministic slot, which is strictly better than every client in the fleet sharing one ladder; the spread only asks for a value that differs between clients, not for a correct one. The or ordering means the wire identity takes over the moment IdentityToken lands, and nothing else in the SDK reads persisted_identity — the comment on the line says so, because that is the constraint that keeps this safe.

A genuinely fresh install has nothing persisted and keeps the exact ladder. That is correct, not a gap: one new client is not a herd.

Tests

dev/test/test_init.lua, reusing the file's existing websocket/timer stubs:

a persisted identity spreads the very first reconnect, before any socket has succeeded — storage holds an identity, the socket errors immediately, and the reconnecting delay equals backoff_delay(1, base, cap, backoff_spread("0xabc…", 1)) exactly. The expectation is computed by calling the real functions, not hardcoded.
which is not the lockstep ladder slot the whole fleet would share — and it differs from the unspread attempt-1 value.
nothing persisted keeps attempt 1 on the exact ladder value — storage = false → exactly 1.
the live wire identity outranks the persisted one once IdentityToken lands / and the persisted identity is genuinely no longer the one being keyed on — storage identity ≠ wire identity, asserted after a successful connect so the or ordering cannot silently flip.

Proof the tests discriminate — reverting the callsite to conn.identity alone:

FAIL dev/test/test_init.lua: FAIL a persisted identity spreads the very first reconnect, before any socket has succeeded: expected 0.5445, got 1
6 passed, 1 failed

Restored: 7 passed, 0 failed. stylua --check clean.

README

The reconnect prose said a cold-start fleet "retries on the exact 1s/2s/4s ladder until one attempt succeeds (see #65)" — now stale. Rewritten to state that the spread seeds from the persisted identity, that the saved identity is not server-confirmed and why that is still fine, and that only a fresh client (nothing persisted, or storage = false) uses the exact ladder.

The connection.backoff_spread row in the API table repeated the stale claim as "nil before an identity exists"; that clause now says it is nil only for a client that has never connected and saved one. connection.lua's own docstring for the pure function was left alone — "returns nil before an identity exists" is a true statement about nil-in/nil-out, and that file was out of scope.

Closes #65. `connect` already spreads each reconnect delay by a deterministic 50–99.95% keyed on the identity, so a server restart does not bring the whole player base back in one instant. But `backoff_spread` was fed `conn.identity`, which is nil until a socket has succeeded — so the one case the spread exists for, a fleet relaunching into an outage, got no spread at all and retried on the flat 1/2/4/8/16/30 ladder in lockstep. `persisted.identity` was already being *written* by `connection.lua`'s `IdentityToken` handler and never read back. This reads it back, for this one purpose: ```lua local persisted_identity = persisted and persisted.identity or nil ... M.connection.backoff_spread(conn.identity or persisted_identity, attempt) ``` ## The judgement call The persisted identity is **not server-confirmed** at that point — nothing has connected yet. Using it anyway is deliberate. The failure mode of a wrong identity is a wrong-but-deterministic slot, which is strictly better than every client in the fleet sharing one ladder; the spread only asks for a value that differs between clients, not for a correct one. The `or` ordering means the wire identity takes over the moment `IdentityToken` lands, and nothing else in the SDK reads `persisted_identity` — the comment on the line says so, because that is the constraint that keeps this safe. A genuinely fresh install has nothing persisted and keeps the exact ladder. That is correct, not a gap: one new client is not a herd. ## Tests `dev/test/test_init.lua`, reusing the file's existing `websocket`/`timer` stubs: - `a persisted identity spreads the very first reconnect, before any socket has succeeded` — storage holds an identity, the socket errors immediately, and the `reconnecting` delay equals `backoff_delay(1, base, cap, backoff_spread("0xabc…", 1))` exactly. The expectation is computed by calling the real functions, not hardcoded. - `which is not the lockstep ladder slot the whole fleet would share` — and it differs from the unspread attempt-1 value. - `nothing persisted keeps attempt 1 on the exact ladder value` — `storage = false` → exactly `1`. - `the live wire identity outranks the persisted one once IdentityToken lands` / `and the persisted identity is genuinely no longer the one being keyed on` — storage identity ≠ wire identity, asserted after a successful connect so the `or` ordering cannot silently flip. Proof the tests discriminate — reverting the callsite to `conn.identity` alone: ``` FAIL dev/test/test_init.lua: FAIL a persisted identity spreads the very first reconnect, before any socket has succeeded: expected 0.5445, got 1 6 passed, 1 failed ``` Restored: `7 passed, 0 failed`. `stylua --check` clean. ## README The reconnect prose said a cold-start fleet "retries on the exact 1s/2s/4s ladder until one attempt succeeds (see #65)" — now stale. Rewritten to state that the spread seeds from the persisted identity, that the saved identity is not server-confirmed and why that is still fine, and that only a fresh client (nothing persisted, or `storage = false`) uses the exact ladder. The `connection.backoff_spread` row in the API table repeated the stale claim as "`nil` before an identity exists"; that clause now says it is nil only for a client that has never connected and saved one. `connection.lua`'s own docstring for the pure function was left alone — "returns nil before an identity exists" is a true statement about nil-in/nil-out, and that file was out of scope.

yuris added 1 commit

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

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

ci / ci (pull_request) Successful in 1m30s

Details

9039fd1f01

yuris added 1 commit

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

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

ci / ci (pull_request) Successful in 20s

Details

a3cf4dcf41

yuris commented

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

Author

Owner

All three findings actioned. The first was a real crash path and the review is right that the rigor already existed one file over.

A non-string persisted identity raised out of the reconnect callback. backoff_spread does #identity then identity:byte(i), so a save file holding identity = 12345 turned every socket drop into a raise from inside schedule_reconnect — i.e. out of the engine's websocket callback. Reproduced exactly as reported:

FAIL dev/test/test_init.lua: ./stdb/connection.lua:681: attempt to get length of local 'identity' (a number value)
6 passed, 1 failed

Fixed by type-checking at the read, following auth.lua:149-152, which guards the same shared slot for the same reason and whose comment names it:

local persisted_identity = persisted and type(persisted.identity) == "string" and persisted.identity or nil

A non-string now degrades to the exact ladder, which is what a client with no identity gets anyway. Two tests added — a number identity degrades to the ladder, not a raise and and neither does a table identity reach the spread — and they fail with the type check removed (output above), so they discriminate.

The sys_storage comment premise was falsified 58 lines below by this patch. Corrected to name both fields, since the conclusion (an empty table yields nil for both) still holds.

backoff_spread's docstring — you are right that I defended only the first clause. "The first connect is a cold start, not a herd" is the policy #65 exists to overturn, and the README this patch rewrites now says the opposite. Replaced with wording that describes the pure function's contract without asserting the policy:

Returns nil only when there is no identity to key on at all — connect passes the persisted one before the first socket has succeeded (#65), so that is a client which has never connected and saved one.

Being in a file the PR otherwise didn't touch was not a good enough reason to leave a false sentence in place.

Suite 7 passed, 0 failed, stylua --check clean.

All three findings actioned. The first was a real crash path and the review is right that the rigor already existed one file over. **A non-string persisted identity raised out of the reconnect callback.** `backoff_spread` does `#identity` then `identity:byte(i)`, so a save file holding `identity = 12345` turned every socket drop into a raise from inside `schedule_reconnect` — i.e. out of the engine's websocket callback. Reproduced exactly as reported: ``` FAIL dev/test/test_init.lua: ./stdb/connection.lua:681: attempt to get length of local 'identity' (a number value) 6 passed, 1 failed ``` Fixed by type-checking at the read, following `auth.lua:149-152`, which guards the same shared slot for the same reason and whose comment names it: ```lua local persisted_identity = persisted and type(persisted.identity) == "string" and persisted.identity or nil ``` A non-string now degrades to the exact ladder, which is what a client with no identity gets anyway. Two tests added — `a number identity degrades to the ladder, not a raise` and `and neither does a table identity reach the spread` — and they fail with the type check removed (output above), so they discriminate. **The `sys_storage` comment** premise was falsified 58 lines below by this patch. Corrected to name both fields, since the conclusion (an empty table yields nil for both) still holds. **`backoff_spread`'s docstring** — you are right that I defended only the first clause. "The first connect is a cold start, not a herd" is the policy #65 exists to overturn, and the README this patch rewrites now says the opposite. Replaced with wording that describes the pure function's contract without asserting the policy: > Returns nil only when there is no identity to key on at all — `connect` passes the persisted one before the first socket has succeeded (#65), so that is a client which has never connected and saved one. Being in a file the PR otherwise didn't touch was not a good enough reason to leave a false sentence in place. Suite `7 passed, 0 failed`, `stylua --check` clean.

yuris merged commit 75300c4e95 into main

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

yuris referenced this pull request from a commit

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

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

yuris referenced this pull request

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

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