fix(sync-service): promote adopted dependency consumers to transaction writes - #4780
Open
alco wants to merge 3 commits into
Open
fix(sync-service): promote adopted dependency consumers to transaction writes#4780alco wants to merge 3 commits into
alco wants to merge 3 commits into
Conversation
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fix/start-dependency-consumer-on-demand #4780 +/- ##
========================================================================
Coverage 60.01% 60.01%
========================================================================
Files 397 397
Lines 43772 43772
Branches 12590 12588 -2
========================================================================
+ Hits 26268 26271 +3
+ Misses 17423 17420 -3
Partials 81 81
Flags with carried forward coverage won't be shown. Click here to find out more. β View full report in Codecov by Harness. π New features to boost your workflow:
|
`Consumer.subscribe_materializer/3` used the default 5-second `GenServer.call` timeout, unlike every other call on the materializer startup path (`await_snapshot_start/3`, `Materializer.subscribe/1`, `wait_until_ready/1`, `new_changes/3`), which already wait with `:infinity` and rely on process monitors for liveness. Bring it in line so the consumer is free to reply whenever it is actually safe to subscribe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uMB7eVy1iXSfY8gqcn5A8
Deferring only the write-unit promotion was not enough. A materializer's
initial history read is bounded by storage's `last_seen_txn_offset`, which
advances only on commit, while a fragment-streaming consumer's
`latest_offset` advances with every fragment it writes. Replying to a
mid-transaction subscription with that offset let the materializer skip
past fragments it could not yet read, silently dropping the head of the
transaction from its index: a two-fragment transaction inserting rows 1
and 2 left the materializer with link values `[2]`.
Park the subscription instead: when `subscribe_materializer` arrives while
a fragmented transaction is pending, stash `{pid, from}` in
`deferred_materializer_subscription` and complete it (monitor, promote,
reply) from `finish_pending_txn/1` at the commit boundary. The consumer is
not marked as subscribed during the wait, so it never calls into the
blocked materializer; the materializer then reads a fully committed
history and receives every later change as a whole transaction.
`State.mark_materializer_subscribed/1` now assumes that invariant and
promotes directly; the separate `maybe_promote_write_unit/1` hook is gone.
A consumer already buffering whole transactions can still be subscribed
mid-transaction, since it has no partial writes on disk.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uMB7eVy1iXSfY8gqcn5A8
β Deploy Preview for electric-next ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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
1 participant
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.
Background
Shape consumers without their own subquery dependencies start with
write_unit: :txn_fragment. Consumers known to be inner shapes start withwrite_unit: :txn, because dependency materializers and outer-shape evaluation require transaction-atomic updates.The startup hint is not sufficient when an already-running consumer is later adopted as a dependency. This can happen in two pre-existing sequences:
In both cases the dependency materializer can subscribe to a consumer that remains in fragment-write mode.
Fix
Treat materializer subscription as the authoritative signal that a consumer is an inner shape and promote it to whole-transaction writes.
If no transaction is in progress, promotion is immediate. If the subscription arrives during a fragmented transaction, that transaction finishes in fragment mode and promotion occurs at its commit boundary. This avoids mixing already-written fragments with a buffered tail of the same transaction.
The existing
is_subquery_shape?startup option remains a fast path for consumers known to be dependencies when they start.Tests
:txn_fragmentto:txn;110 passed, 1 excludedacross ShapeCache, consumer state, and consumer tests.Stack
This PR is stacked on #4779 and should be reviewed against
fix/start-dependency-consumer-on-demanduntil that PR merges.