fix(filler): stop deriving the processing state from a one-way flag - #199
Merged
Conversation
contract_readers.live goes true the first time a reader reaches head and nothing ever writes it back, so seeding the processing state from it at startup puts a reader that once reached head into head mode however far behind it restarts. The in-process promotion cannot correct that, since it only fires while the state is still catchup, and the first commit writes the flag out again. Every restart re-arms it. Head mode is not free while a reader replays a backlog. Before 2.2.1 it published every trace and delta to redis inside the commit path, and it still forces a checkpoint write on each commit. Starting in catchup costs nothing instead, because process() re-checks the same head-distance predicate on every block and promotes on the first one that qualifies, ahead of that block's commit and of its notifications. A reader that really is at head is therefore in head mode from its first block. The flag is still written, since reconcile reads it as a liveness hint, but nothing derives behaviour from it and getReaderPosition no longer returns it. Signed-off-by: Rob Konsdorf <rob@facings.io>
There was a problem hiding this comment.
Pull request overview
Starts fillers in catch-up mode instead of deriving state from the persistent one-way live flag.
Changes:
- Initializes processing state as
CATCHUP. - Removes
livefrom reader-position queries. - Adds regression coverage for restarts with a stored live flag.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/filler/receiver.ts |
Initializes processing in catch-up mode. |
src/filler/receiver.test.ts |
Tests restart behavior with a stored live flag. |
src/filler/database.ts |
Removes the unused live flag from position results. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Merged
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
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.
Why
contract_readers.livegoes true the first time a reader reaches head, and nothing ever writes it back.startProcessing()seeded the processing state from it, so a reader that once reached head started in head mode however far behind it restarted. The in-process promotion could not correct that, because it only fires while the state is still catchup, and the first commit wrote the flag out again. Every restart re-armed it. This has behaved the same way at every tag from 1.7.5 to 2.3.1.Head mode is not free while a reader replays a backlog. Before 2.2.1 it published every trace and delta to redis inside the commit path, which is the defect #161 fixed from the other end, and it still forces a checkpoint write on every commit.
Starting in catchup costs nothing, because
process()re-checks the same head-distance predicate on every block and promotes on the first one that qualifies, ahead of that block's commit and of its notifications. A reader genuinely at head is in head mode from its first block, so no notification and no checkpoint behaviour changes for a caught-up filler.getReaderPosition()no longer returns the column either. Leaving it there with no consumer is what invites the latch back.reconcileis unaffected: it readscontract_readerswith its own query, and its independent 60 second staleness check is what actually guards it.Risk
This changes runtime behaviour on every filler, so it is deliberately separate from #198. The window it touches is the first block after a start. A reader at head promotes on that block before anything is committed or published, and a reader behind head was already meant to be in catchup and now is.
Validation
pnpm testgoes from 419 passing to 420, 37 pending unchanged. The added test assertssetStateis called withCATCHUPwhile the stored flag reads true, and it fails against the old line: restoring the seed and re-running gives 419 passing and 1 failing on exactly that test.pnpm check-typesandpnpm lintare clean.