You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Our mz_join_core is a fork of differential's JoinCore::join_core and inherited a correctness bug fixed upstream in TimelyDataflow/differential-dataflow#802 (issue #801).
The operator skipped incoming batches by testing batch.lower() against the mutable acknowledged frontier. advance_upper can advance acknowledged past an in-flight batch when trace merges consolidate the batch's updates away, e.g. an add/remove pair collapsing once logical compaction equates their times. The trace is legitimately empty there only for readers at or beyond the compaction frontier, while our consumers may read finer times where the batch's updates are still real. Testing against acknowledged therefore dropped such batches and silently lost updates.
The fix captures fixed preload_upper frontiers at start-up and gates the skip on them, and advances acknowledged only when a batch sits at or beyond it. This is a one-to-one port of the upstream change.
No regression test: deterministically reproducing the race requires trace merges to consolidate an add/remove pair while a batch is in flight and advance_upper to then jump ahead, which is timing-dependent and not reliable in testdrive/sqllogictest. Upstream #802 added no direct join test either. Correctness rests on mirroring the upstream reasoning.
Note: the alternate DifferentialDataflow join implementation calls upstream join_core from crates.io differential-dataflow 0.24.0, which still carries the bug and will be fixed by a future dependency bump.
`mz_join_core` skipped incoming batches by testing `batch.lower()`
against the mutable `acknowledged` frontier. `advance_upper` can move
`acknowledged` past an in-flight batch when trace merges consolidate the
batch's updates away, for example an add/remove pair collapsing once
logical compaction equates their times. The trace is legitimately empty
there only for readers at or beyond the compaction frontier, but our
consumers may read finer times where the batch's updates are still real
and unaccounted for. Testing against `acknowledged` therefore dropped
those batches and silently lost updates.
Capture fixed `preload_upper` frontiers at start-up and gate the skip on
them instead, and only advance `acknowledged` forward when a batch sits
at or beyond it. This mirrors the upstream fix in
TimelyDataflow/differential-dataflow#802 (issue #801), which our fork of
`JoinCore::join_core` had inherited.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Our
mz_join_coreis a fork of differential'sJoinCore::join_coreand inherited a correctness bug fixed upstream in TimelyDataflow/differential-dataflow#802 (issue #801).The operator skipped incoming batches by testing
batch.lower()against the mutableacknowledgedfrontier.advance_uppercan advanceacknowledgedpast an in-flight batch when trace merges consolidate the batch's updates away, e.g. an add/remove pair collapsing once logical compaction equates their times. The trace is legitimately empty there only for readers at or beyond the compaction frontier, while our consumers may read finer times where the batch's updates are still real. Testing againstacknowledgedtherefore dropped such batches and silently lost updates.The fix captures fixed
preload_upperfrontiers at start-up and gates the skip on them, and advancesacknowledgedonly when a batch sits at or beyond it. This is a one-to-one port of the upstream change.No regression test: deterministically reproducing the race requires trace merges to consolidate an add/remove pair while a batch is in flight and
advance_upperto then jump ahead, which is timing-dependent and not reliable in testdrive/sqllogictest. Upstream #802 added no direct join test either. Correctness rests on mirroring the upstream reasoning.Note: the alternate
DifferentialDataflowjoin implementation calls upstreamjoin_corefrom crates.iodifferential-dataflow0.24.0, which still carries the bug and will be fixed by a future dependency bump.🤖 Generated with Claude Code