Offsets for a batch the sink refused are no longer committed on shutdown - #279
Merged
Conversation
turbolytics
added a commit
that referenced
this pull request
Sep 13, 2026
…ates CHANGELOG.md: main now keeps an Unreleased section. serve's entries join its Added list, and serve's limits sit under Known limits, rather than under a v1.3.0 heading of their own. The version is named when it is tagged. internal/errs/errs_test.go: both sides appended a test at the end of the file. Both are kept. internal/validate/validate.go merged without a conflict and with a bug: #273's checkDrainDeadline ran on every file, so a serve file reported pipeline.drain_deadline: pass for a pipeline it does not have. A check that could not apply must not read as a pass. Each file now gets its own checks: serve rules for a serve file, the drain deadline for a pipeline. TestValidateServe_AValidServeFilePasses now asserts no pipeline check reports on a serve file; it failed on the merged code and passes now. docs/coverage/matrix.md merged as text into a page that no longer matched its status files, and test_the_committed_page_is_current failed. make coverage-page regenerated it: 39 features, cli.serve covered at unit and release, 0 gaps. go vet ./..., go test -short -race ./... and pytest tests/tooling pass on the merge.
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.
A stateful pipeline loses rows on main whenever its sink refuses a batch. The process exits with the sink's error, but the offsets for that batch become durable, and a restart resumes past rows nothing wrote.
The bug
runsyncs state twice after the loop returns, before and after the table managers' final poll. Those syncs save the positions held in memory.A second path loses rows the same way. A cancel that lands exactly as a batch fills returned a clean stop without writing the batch, exited 0, and the shutdown committed its positions.
Found by an adversarial review of #273. It predates #273.
The fix
ConsumeLoopresets the in-memory positions to that copy, so a later commit saves nothing new.Marks.Resetis the one operation allowed to move a position backwards. It reuses its maps, so recording the copy on every commit allocates nothing new per commit.The harness
The new invariant
pipeline.shutdown.commits_only_deliveredis enforced. Every earlier pipeline check stopped when the loop returned, so none of them could see this bug.run's two shutdown syncs, and fails if any durable position covers a message the sink did not acknowledge.boundary-cancel, lands the cancel as the batch fills. Every pipeline invariant now runs on it.Against main's turbine, the new harness fails
pipeline.shutdown.commits_only_deliveredon a failed flush, and failsafter_flush,nothing_on_failureandstate.with_offsetson the boundary-cancel trigger.Verified
go test -short -race ./...,go vet, gofmt andpytest tests/toolingpass.Coverage status files follow from this PR's CI report artifacts.