Skip to content

Set up repl.lag once and apply options at every level - #196

Draft
Areson wants to merge 3 commits into
Areson/heartbeat-reader-leakfrom
Areson/repl-lag-level-setup
Draft

Areson wants to merge 3 commits into
Areson/heartbeat-reader-leakfrom
Areson/repl-lag-level-setup

Conversation

@Areson

@Areson Areson commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #189.

Lag.Prepare never assigned configured, so its per-level loop had three bugs (all also on main):

  • Every level re-ran the full writer setup, and a plan with different writers at different levels wasn't rejected.
  • With the blip writer, only the first level (random map order) got report-no-heartbeat. Other levels reported repl.lag.current = -1 instead of dropping it.
  • The heartbeat reader was made before repl-check was set, so it never selected @@<repl-check> and report-not-a-replica did nothing.

Now Prepare sets up the writer once, from the first level (by name) that sets pfs or blip, or else by auto-detecting once. Levels with no writer or auto use that writer, because inheritance can leave them unset. Only pfs at one level and blip at another is rejected, whatever the map order. Prepare also sets report-no-heartbeat, report-not-a-replica and default-channel-name at every level. The reader gets repl-check from the level options.

This also documents the cleanup-on-error contract on blip.Collector.Prepare: a collector releases what it started before returning an error, and returns a nil cleanup. The engine call from #189 stays as a documented backstop. Lag already did this.

BlipReader.Stop now waits for the reader to return (separate commit). Before, a heartbeat query running at stop could report status after the loader removed the monitor's status, and the removed monitor reappeared in /status/monitors. Stop can now block up to ReadTimeout (2s). Monitor.Stop's db.Close already waited for that query, so the wait moves earlier rather than adding time. It fits in #190's 3s default stop timeout, and exceeding that timeout only reports error:stop. A reader stopped before its Start goroutine runs doesn't start, so Stop doesn't block on it. TestReader now stops its reader.

The new tests run without MySQL, except TestPrepareAutoAndPFSOnMySQL80. Each fails on the old Prepare.

🤖 Generated with Claude Code

Lag.Prepare never assigned `configured`, so every level re-ran the full
writer setup and mixed writers were never rejected. With the blip
writer, only the first level (in random map order) got
report-no-heartbeat, so other levels reported -1 instead of dropping
it. And the heartbeat reader was made before repl-check was set, so it
never selected @@<repl-check> and report-not-a-replica did nothing.

Prepare now sets up the writer once, from the first level, rejects a
different writer at another level (an unset writer is auto), and sets
the per-level options at every level. The reader gets repl-check from
the level options.

Document on blip.Collector.Prepare that a collector must release what
it started before it returns an error, with the engine calling a
returned cleanup func only as a backstop. Lag already did this.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ian Oberst and others added 2 commits October 22, 2026 14:07
Prepare compared each level's raw writer option with the first level's,
so a plan with no writer (auto) at one level and writer: blip at another
failed to prepare. Plans have that because levels inherit options only
from lower levels whose frequency divides theirs. Which side the error
named also depended on map order.

Now Prepare sets up the writer from the first level, by name, that sets
pfs or blip, else it auto-detects once from the first level. Levels with
no writer or auto use that writer. Only pfs at one level and blip at
another is an error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stop closed stopChan but didn't wait for run to return. If a heartbeat
query was running, run reported status after the query returned, which
could be after the monitor loader removed the monitor's status. The
removed monitor then reappeared in the monitor status with one stale
heartbeat-reader component.

Stop now waits for run to return, without holding the lock that run
uses to update lag. That takes up to ReadTimeout if a query is
running. Start doesn't start a stopped reader, so Stop doesn't wait
for a reader that never started, which happens when repl.lag stops
the reader before its Start goroutine runs.

TestReader now stops its reader. It leaked the reader, which raced
with the next run's ReadErrorWait write under -count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant