Repository navigation
Fix SQL sink final flush and handler errors when the stream ends - #1011
Merged
maoueh merged 1 commit intoOct 29, 2026
Merged
Conversation
This was referenced Oct 23, 2026
sduchesneau
requested review from
maoueh
and removed request for
UlysseCorbeil
October 23, 2026 19:23
aggris2
force-pushed
the
fix/sql-sink-stop-and-handler-errors
branch
from
October 23, 2026 20:46
c433edb to
072ed60
Compare
maoueh
force-pushed
the
fix/sql-sink-stop-and-handler-errors
branch
3 times, most recently
from
October 26, 2026 17:36
e6d6e76 to
3c45683
Compare
…server Sink library: Sinker.AwaitRun(ctx) waits for Run to return and gives its first failure. A stop terminates the sinker right away while Run still finishes its completion handler, so its failure cannot be the termination error anymore. The SQL changes sinker now relies on it instead of tracking its own run, and the CLI waits up to 45s for the final flush on a stop. A handler failure while stopping is taken as caused by the stop: a cancelled database call does not always wrap context.Canceled. A stop that comes before the sinkers wired their termination callbacks now still cancels the stream. The fake Substreams stream server and its response builders move to a new sink/sinktest package, used by the sink library tests and the SQL integration suite. The restart/undo test moves to that suite, against a Postgres testcontainer, with a case for the undo buffer cursor.
maoueh
force-pushed
the
fix/sql-sink-stop-and-handler-errors
branch
from
October 29, 2026 13:31
3c45683 to
6393b7f
Compare
maoueh
approved these changes
Oct 29, 2026
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.
Builds on #1007. Until that one is merged, this PR also shows its commit.
A few problems with how the sink finishes, found while testing restarts and failovers:
The final flush prunes undo history. When the stream ends (stopped, or at the stop block), the completion flush passes the stream's latest block as the final block. That deletes the history rows of blocks that are still reversible. If one of them is reorged after the sink restarts, the undo has nothing to revert and the orphaned rows stay. With
--undo-buffer-sizeit also saves the stream's cursor, which is ahead of the blocks still in the buffer, so those get skipped on restart.A handler error wrapping
io.EOFends the run as if it completed. The stream loop checkserrors.Is(err, io.EOF)first, so a lost database connection (failed to begin db transaction: EOF) looks like the stop block was reached. A live sink then exits with status 0 and isn't restarted on failure.Handler errors are swallowed while stopping. Once the context is cancelled, any error is ignored and the run ends as completed, so the completion handler can flush a block the handler didn't finish.
Also,
Revert's deferred rollback never ran: it checked a differenterr.Fix:
Sinker.LastReceivedCursorgives the stream's.Added
TestSinker_CompletionFlush,TestUndoAfterRestart_Postgres(needsPG_DSN), andsink/sinker_stream_test.go, which runs the sinker against a fake stream: completion with an undo buffer, a handler failing while stopping, a handler returningio.EOF. All fail on the base and pass with this change. The existing Postgres integration suites pass.