perf(ui): restore rooms in ~4 s, not 12-18 s, on the first load after a re-key

[sanity]

Problem

After the #703 re-key, the first load on try.freenet.org took 20+ s to show rooms (#809), while the node executed each delegate request in about 10 ms. The issue listed three candidate waits but had not tied the delay to one.

I measured it with a rehearsal: a pre-#703 UI (263770e, delegate V36) seeds rooms on an isolated gateway, then main's UI (0dd55bb) loads. A bare node did not reproduce it (rooms in 1.9 s). Two production conditions did:

• the 70 delegate registrations from the try.freenet.org node (so legacy generations answer instead of reporting "missing"), plus the hosted binary and its compile cache;
• a delay proxy adding 60 ms each way.

With those, rooms appeared at 11.7 s, or 18.2 s at 100 ms each way. The console timeline showed what held them:

1. The migration journal's begin read every predecessor's markers one round trip at a time: done, then in-progress, for 33 generations. That is 66 serial reads, and the sweep's merge waits for begin. This was most of the delay, and it grows with generation count × RTT.
2. The sweep asked the legacy generations oldest first. The newest generation, which holds the rooms, sat in the node's serial queue behind all the others.
3. A legacy generation's room slots were fetched one round trip per room.

A fourth cause turned up while measuring the fix. Every delegate request on a node, from every client and for every delegate, shares one admission queue per priority tier, capped at 100 (MAX_QUEUED_PER_CONTRACT, freenet-core fair_queue.rs). The sweep sent 99 requests at once. The node rejects a request past the cap with an error that names no delegate ("no response received from handler", logged only at debug), so the waiter for that request runs out its whole 10 s timeout. A first version of this fix added concurrent reads alongside the sweep and got 26 s. On the shared hosted node, several users loading at once would reach this from the sweep bursts alone.

Not on the visible path in any run: REGISTER_ACK_TIMEOUT_MS (the register was acked in 30 ms), REKEY_SEED_WAIT (rooms were already listed when it started), and the walk's reply timeouts (the walk runs after the load settles).

Approach

• begin prefetches the markers in concurrent waves of 16 (RiverSuccessorIo::prefetch_markers), only when no journal is stored yet, which is the first load after a re-key. Only definitive answers are cached. A silent or failed read is read again live and still refuses begin, so silence is still never treated as absence (constraint 1). A cached value is used at most once and is dropped when its key is written, so the crate's marker read-back sees the write. With a journal stored, begin runs off the visible path, and prefetching there made steady-state loads about 0.18 s slower, so it is skipped.
• The sweep goes newest first, in waves of 4 generations (12 requests). It sends the first wave and returns, as the whole sweep used to, so the loader's state write still lands before any reply. The remaining waves go out from a background task that holds a load worker. Each wave goes out once the previous one has been answered (a reply to one of the sweep's own requests, or an error naming its delegate), or after 2 s. A pacing sweep stops if a newer sweep starts or its load attempt changes (a reconnect or Retry). The new attempt may then sweep again, at most twice a session. Arrival order still decides nothing; merges are ranked by source generation (Lost IDs and rooms to River Update across two nodes. #527).
• Legacy room slots are fetched in waves of ROOM_LOAD_CONCURRENCY, one generation at a time, with duplicate keys dropped. A compile-time check keeps one client's first-load burst (a sweep wave, a slot wave and a marker wave) at or below half the node's shared queue.

Testing

Rehearsal results, harness and logs on nova at ~/code/tmp/river-809/m/; each figure is one run unless noted:

scenario	before (0dd55bb)	after (7bd702f, final head)
6 rooms, 60 ms each way, production-like node	11.53 s (back to back); 11.6-11.7 s in earlier runs	3.84 s
6 rooms, 100 ms each way	18.24 s	5.83 s
bare node, missing legacy delegates, no delay	1.93 s	2.11 s
later loads (bare node / 6 rooms at 60 ms)	1.34-1.52 s at 60 ms	0.40-0.45 s bare; 1.41-1.45 s at 60 ms (415e042)

Intermediate builds measured the same way: 3.87 s (70498e6, 4 rooms), 4.64 s (12 rooms, before the fresh-journal gate), and 4.07 / 3.97 s (5e9544b, back to back with base). Each figure is a single run unless noted.

The rest of the 4 s is mostly the journal flagging the import before the merge (about 9 round trips, required by the crate's contract) and begin itself.

New tests:

• begin_reads_markers_in_waves_not_one_round_trip_each: begin over 33 generations must not take more round trips than over 3, beyond the extra waves. Fails before the fix: 69 round trips against 9.
• begin_still_refuses_on_a_marker_that_stays_silent: constraint 1 holds with the prefetch.
• a_prefetched_marker_does_not_outlive_a_write.
• begin_prefetches_only_before_the_journal_exists.
• a_sweep_wave_completes_when_every_request_is_answered, a_typed_missing_error_answers_its_sweep_request (the node's real error texts), only_sweep_replies_count_toward_a_wave, the_sweep_plan_is_newest_first_bounded_and_complete.
• Source pins: no wave is awaited before the sweep returns, and the legacy slot fan-out shape.

cargo test -p river-ui --bins: 1324 passed (1330 with example-data,no-sync), on 7bd702f.

Residual

The issue also asked to stop waiting out timers on known misses in general. This PR does that for the waits measured on the visible path. The register-ack wait and the walk's reply timeouts were measured off that path and are left for a follow-up.

The journaled walk's pre-warm still sends one ListRequest per generation at once (33). It runs after the load settles, so it is off the visible path. It does draw on the same shared queue. The shared 100-request cap, and the fact that its rejection names no delegate, are on the freenet-core side. When the node does refuse a sweep request that way, that wave waits its 2 s bound.

Follow-up for the remaining timers: #811.

Closes #809

[AI-assisted - Claude]

https://claude.ai/code/session_01Mynjd42gMDNsNSxwbJWKu3

[sanity]

Comprehensive PR Review: #810 (round 1)

Summary

• Type: perf / fix
• Linked issue: #809
• Review tier: Full (delegate migration, async concurrency)
• HEAD reviewed: 70498e6
• Reviewers run: code-first, testing, skeptical and big-picture (freenet agents), plus a migration-safety / data-loss lens.
• External model: codex review failed twice with "You've hit your usage limit ... try again at Oct 22nd". Per the review rules, the five Claude lenses above stood in for it.

Findings and what was done (fixes in fc2d5ab and 7da0a3d)

Must Fix

1. The load state reverted from Loaded to Loading (response_handler.rs:853).
   • Found independently by the skeptical and migration lenses; I verified it.
   • The loader awaited the paced sweep and then wrote Loading. By then the first wave's merge had already written Loaded, and nothing restored it. Identity import stayed off until the 60 s backstop.
   • Fixed. The sweep sends its first wave and returns, exactly as the whole sweep used to. Later waves run from a background task that holds a LoadWorkerGuard. A test now pins that no wave is awaited before the sweep returns.

Should Fix

2. Slot replies ended waves early, and several generations' slot fan-outs could exceed the node's 100-request queue. Fixed.
   • Only the sweep's own replies count toward a wave: the rooms_data and outbound_dms GETs, the ListRequest, or an empty reply (is_sweep_reply, tested).
   • Legacy slot fan-outs run one generation at a time (LEGACY_SLOT_FETCH).
   • A compile-time check keeps one client's burst (sweep wave + slot wave + marker wave) at or below half the cap.
3. The global wave state broke when two sweeps overlapped (a Retry or recovery re-fire). Fixed. Waves are now kept per sweep, by token.
4. A reconnect mid-sweep could skip the older generations for the rest of the session. Fixed. A transport failure in a later wave clears the once-per-session guard, so the next load attempt sweeps again. The journaled walk covers them either way.
5. Up to 18 s of Loading for new users if waves stay silent.
   • Mitigated. The real typed Missing and the rate limiter's refusal now answer a wave (tested with the node's actual error texts).
   • Measured on a bare node where every older generation is missing: 1.75 s.
   • A wave still waits its 2 s bound when the node turns requests away with an error naming no delegate. That case is a residual.
6. Stale docs. Fixed: the fire_legacy_migration_request doc, the ListRequest comment, the module comment on generation authority, and two room_data.rs test comments that described oldest-first dispatch.
7. Source-pin tests only. Partly fixed. Added behavioural tests for the wave plan (newest first, complete, bounded), reply classification, and real error texts. The legacy slot fan-out is still pinned by source, matching its current-delegate twin.

Consider

• Extra journal read in begin_with: fixed. The read is handed to begin's first journal load, so it costs no round trip, and begin sees the journal the markers were read with. That also closes the migration lens's C1.
• No dedup in the legacy fan-out: fixed.
• A second tab writing a marker between the prefetch and its use: accepted. Markers only move forward, so a stale read can only under-report. The migration lens checked that the crate's journal merge keeps another tab's in-progress flag. The window is shorter than with the old serial reads.
• The 12-room row in the description came from a build without the fresh-journal gate: the description is updated with re-measured numbers on fc2d5ab.

Data-safety verdict (migration lens)

No path found that loses rooms, identities, DMs or tombstones. None marks a predecessor resolved or imported when it was not, or seals one wrongly. The prefetch caches only definitive replies whose key matches, so silence is still never treated as absence. Newest-first ordering does not change merge results: merges are ranked by generation, not by arrival.

Verdict

State: Needs Changes. Re-review required after the fixes, because the Must Fix touched the migration and concurrency surface. A fresh round runs on 7da0a3d.

[AI-assisted - Claude]

[sanity]

Comprehensive PR Review: #810 (round 2, re-review after round-1 fixes)

• HEAD reviewed: 7da0a3d
• Tier: Full
• Lenses: code-first, testing, skeptical and big-picture (freenet agents), plus migration-safety / data-loss.
• External model: codex review is still unavailable ("usage limit ... try again at Oct 22nd"), so the Claude lenses stood in again.

Round-1 findings

All five lenses confirm the round-1 Must Fix is fixed: the loader's state write no longer lands after the first wave's merge. They also confirm the per-sweep wave tokens, sweep-only reply counting, the serialized slot fan-out and the journal handoff.

The migration lens checked the journal handoff against freenet-migrate 0.7.0. begin loads the journal twice. The one-shot handoff feeds only the first load (start_journal); store_merged's read-merge-write still reads fresh, so another tab's journal is merged in. No path was found that loses rooms, identities, DMs or tombstones, or that resolves or seals a predecessor wrongly.

Round-2 findings and what was done (fix in 415e042)

Should Fix

1. The background waves ignored load attempts. Raised by the skeptical, migration and code-first lenses.
   • After a reconnect or Retry they kept pacing, alongside the walk (whose quiescence wait no longer counted them) or alongside a second sweep. A stale sweep could also clear the once-per-session guard under a newer one.
   • Fixed. Each sweep has an epoch. Before each later wave it stops if a newer sweep started, leaving the guard to that sweep. It also stops if its load attempt changed, and then clears the guard so the new attempt sweeps from the start. Only the newest sweep clears the guard after a transport failure. A new attempt may start a sweep while an earlier attempt's sweep is still pacing, but never over its own. Tested: a_pacing_sweep_yields_to_newer_sweeps_and_attempts.
2. A wave lost with a dropped socket went unnoticed if the next send succeeded on the new socket. Fixed by 1: a reconnect is a new attempt, so the old sweep stops and the new attempt re-sweeps.
3. The pacing hooks, the slot-fetch lock and dedup, the per-sweep registry and the one-shot journal handoff were untested. Fixed:
   • sweep_pacing_hooks_are_wired;
   • the legacy fan-out pin now requires LEGACY_SLOT_FETCH, taken before the first enqueue and released after the last join_all, plus the dedup;
   • sweep_waves_are_kept_per_sweep;
   • begin_reads_the_journal_once_for_its_start_and_fresh_for_its_store, which expects exactly 2 journal reads.
4. Four comments still described oldest-first dispatch as current (direct_messages.rs, three in room_data.rs), plus the "sequential fetch loop" comment and the idle-timer doc. Fixed.

Consider

• Errors for non-sweep requests that name a legacy key still count toward a wave, and overlapping sweeps share answers. This affects pacing only and is left as is. An error carries no request id, so it cannot be attributed more precisely.
• The budget check covers one sweep wave, one slot wave and one marker wave. Since 1, sweeps no longer overlap. A wave that timed out may leave requests queued while the next goes out, which is bounded by the 2 s wait.
• The sweep's load worker keeps Loading, which defers identity import, for the paced sweep's duration. About one round trip per wave on a responsive node; 2.02 s in total on the bare-node rehearsal.
• Stale prefetched markers from a concurrent session: documented on prefetch_markers. Markers only move forward, so an early read can only under-report.
• "Closes #809": kept. The issue's second ask (stop waiting out timers on known misses) is handled for the waits on the visible path that measurement found. The remaining timers (the register-ack wait, the walk's reply timeouts) were measured off that path. I'm filing a follow-up for them and for the node-side queue cap.

Re-measured on 415e042 (rehearsal on the hosted binary and its compile cache, production's delegate registrations)

• 6 rooms, 60 ms each way: 3.86 s (11.68 s before)
• 6 rooms, 100 ms each way: 5.76 s (18.24 s before)
• bare node: 2.02 s (1.93 s before)
• later loads: 1.41 / 1.45 / 1.41 s

cargo test -p river-ui --bins: 1324 passed (1330 with example-data,no-sync).

Verdict

State: Needs Changes. The round-2 fixes touch concurrency, so round 3 runs on 415e042.

[AI-assisted - Claude]

[sanity]

Comprehensive PR Review: #810 (round 3, re-review after round-2 fixes)

• HEAD reviewed: 415e042
• Tier: Full
• Lenses: skeptical, code-first, migration-safety.
• External model: Codex is still unavailable (usage limit until Oct 22), so Claude lenses again.

Findings on 415e042

No Must Fix, and no data-loss path. The migration lens traced every caller of fire_legacy_migration_request. Each one fires only over an empty current delegate (#253), and the guard only limits repeat probes, so clearing it cannot enable a fire on a path that could not fire before.

Should Fix, all fixed in 5e9544b:

1. A reconnect during a sweep's pacing cleared the once-per-session guard with no limit. If the sweep itself kept dropping the socket, that meant re-probing on every reconnect. The guard's own doc (skeptical S1) exists to prevent exactly that.
   • Fixed: at most MAX_SWEEP_REARMS (2) re-arms per session.
2. "Never over its own" was false. A recovery re-fire, or a Retry, clears the guard, which let a second sweep start over the same attempt's pacing one (code-first 1).
   • Fixed: sweep_may_start refuses whenever this attempt's own sweep is pacing.
3. The running marker had gaps. It was set only after the first wave and written even by a superseded sweep (skeptical C1/C2).
   • Fixed: it is set before the first send. Every stop path goes through stop_sweep, where only the latest sweep clears the marker or the guard (pure effects, tested). A test also pins that nothing in the task writes either directly.
4. A sweep superseded mid-wave still sent the rest of that wave, and a sweep whose attempt changed during the first wave still spawned a task (skeptical C3/C4).
   • Fixed: it stops sending mid-wave, and spawns no task in that case.
5. Stale docs: the guard docs said only Retry clears it and called it part of the #253 gate.
   • Fixed: they now say the guard limits re-probes and #253 is enforced by the callers.
6. "New attempt sweeps from the start" overstated it. The new attempt sweeps only if its load reaches the sweep.
   • Fixed: the comment now says "may sweep". The journaled walk covers the rest.

Consider (not changed):

• The pacing-wiring tests are partly source pins (no wasm runtime in the unit tests).
• SWEEP_WAVES is process-global in one test. It uses a unique key and tokens, so nothing interferes with it today.
• A dead-code path (load_rooms_from_delegate) could fire the sweep on a missing rooms_data alone. It predates this PR and is unreachable.

Re-measured on 5e9544b (back to back with the base build, same conditions)

• 6 rooms, 60 ms each way: base 11.71 / 11.61 s, fix 4.07 / 3.97 s.
• Bare node: 2.01 s.

Tests: cargo test -p river-ui --bins 1324 passed; 1330 with example-data,no-sync.

Follow-up for the timers off the visible path: #811.

Verdict

State: Needs Changes. These fixes touch concurrency, so round 4 runs on 5e9544b.

[AI-assisted - Claude]

[sanity]

Comprehensive PR Review: #810 (round 4, re-review after round-3 fixes)

• HEAD reviewed: 5e9544b
• Tier: Full
• Lenses: skeptical, code-first, migration-safety.
• External model: Codex is still unavailable (usage limit until Oct 22).
• CI: green on 5e9544b.

Findings on 5e9544b

No Must Fix, and no data-loss path. The migration lens confirmed three things:

• A generation that a stopped sweep skips is never stranded. The sweep writes no done markers; the journaled walk runs on every load while any predecessor is pending, and a walk refused mid-attempt re-arms for the next attempt.
• #253 holds at every caller.
• Nothing else in the round touched data paths.

Fixed in 16888b5:

1. (skeptical M1) The cap could be bypassed.
   • A reconnect whose new attempt reached the sweep before the old sweep noticed started a full re-sweep (as OverEarlier) without touching the re-arm cap. The cap did not bound it.
   • Now that path spends from the same per-session budget (sweep_start and take_sweep_rearm).
2. (skeptical M2, code-first 1, migration 1) No re-arm when the attempt changed during the last wave. That wave has no next wave to notice the change, so the stop ran without re-arming.
   • The task's end, and the path where the first wave is the whole sweep, now re-arm when the attempt changed.
3. (skeptical L1) A Retry spent a re-arm when it had already cleared the guard. Now a re-arm is spent only when the guard is still set.
4. (code-first 2) The all-sends-failed first wave bypassed the latest-sweep rule. It now clears the guard only from the latest sweep. It still spends no re-arm, because the probe never left (the #397 rule).
5. (migration 2) The guard doc now names the #352 recovery as the one caller that fires over a non-empty current delegate, and says why that is safe: the per-room load read every slot, and saves are per-room CAS.
6. (code-first 3) Duplicate assertion in the sweep test removed. The stop effects are now tested with the new guard-state input.

Not changed: (code-first 4) the re-arm cap's scope (a transport failure mid-sweep also spends one) is documented on MAX_SWEEP_REARMS. Retry and the recovery take none.

Tests: cargo test -p river-ui --bins 1324 passed; 1330 with example-data,no-sync.

Verdict

State: Needs Changes. Round 5 runs on 16888b5.

[AI-assisted - Claude]

[sanity]

Comprehensive PR Review: #810 (round 6, final)

Summary

• PR: perf(ui): restore rooms in ~4 s, not 12-18 s, on the first load after a re-key
• Type: perf / fix
• Linked issues: closes #809. Follow-up for the timers off the visible path: #811.
• Review tier: Full (delegate migration, async concurrency)
• HEAD: 7bd702f
• CI on that SHA: build, check-delegate-migration, check-room-contract-migration, check-pointer-freshness, ui-playwright-tests and license/cla all pass.
• Reviewers run: six rounds of independent Claude lenses.
  • Round 1 and round 2: code-first, testing, skeptical, big-picture and migration-safety.
  • Rounds 3 to 6: skeptical, code-first and migration-safety.
• External model: codex review was unavailable for every round ("usage limit ... try again at Oct 22nd"; retried once), so the Claude lenses stood in for it.

Round 6 (on f4e1da6)

No Must Fix or Should Fix findings from any lens.

• Skeptical: verified the last-wave await and the latest-sweep guard reset against the walk, the backstop and the idle timer. The worst-case sweep (about 18 s) stays under both 60 s limits.
• Migration-safety: confirmed the latest-sweep gate covers the old #397 concern (a stale probe reopening the guard for a new attempt), that #253 holds at every caller, and that no predecessor can go unmigrated.
• Code-first: found four stale comments. They were fixed in 7bd702f (comment and doc text only: the delta since f4e1da6 changes no code), along with river-publish.md step 4.

Data safety across all rounds

No path was found that loses rooms, identities, DMs or tombstones, or that resolves or seals a predecessor wrongly.

• The marker prefetch caches only definitive replies whose key matches, so silence is never treated as absence.
• The journal handoff is one-shot, so store_merged still reads fresh.
• Merges stay ranked by generation, not by arrival.
• A generation the sweep skips stays pending for the journaled walk.

Residuals (disclosed in the description and #811)

• A wave waits its 2 s bound when the node turns a request away with an error that names no delegate.
• The walk's pre-warm is not paced.
• A reconnect while the walk is already running leaves rooms unrestored until reload. This exists on main too.

Final measurements (7bd702f, rehearsal on the hosted binary and its compile cache, production's delegate registrations)

scenario	before	after
6 rooms, 60 ms each way	11.53 s	3.84 s
6 rooms, 100 ms each way	18.24 s	5.83 s
bare node	1.93 s	2.11 s

Verdict

State: Ready to Merge (review complete on the current HEAD, CI green).
HEAD SHA reviewed: 7bd702f

Not merged and not published by me. River UI publishing is a separate step for whoever releases.

[AI-assisted - Claude]