Skip to content

DDIR: preserve timestamp order across Corgi chunk boundaries - #862

Merged
frankmcsherry merged 1 commit into
master-nextfrom
codex/ddir-corgi-merge-horizon-fix
Sep 7, 2026
Merged

DDIR: preserve timestamp order across Corgi chunk boundaries#862
frankmcsherry merged 1 commit into
master-nextfrom
codex/ddir-corgi-merge-horizon-fix

Conversation

@frankmcsherry

Copy link
Copy Markdown
Member

The Corgi chunk merger added in #861 can emit timestamps out of order when an
equal (key, value) group continues across an input chunk boundary.

For identical keys/values (brackets are chunk boundaries):

left:     [0] [2]
right:    [1, 3]
actual:   [0, 1, 3, 2]
expected: [0, 1, 2, 3]

survey_groups groups by key/value, not timestamp. The Both path merges
timestamps until one side's range ends, then copies the other range's suffix.
If the exhausted range also ends its whole input chunk, that suffix must wait
for the next chunk: it can contain timestamps beyond the shared merge horizon.

Stop at that boundary and return the unconsumed suffix through the existing
push-back path. The batched key/value survey is unchanged. Eight implementation
lines; no dependency, batch-size, query, or server-protocol changes.

The regression uses the plain ingest ChunkMerger. A later trace advance
can re-sort equal-value timestamps and hide the violation, so checking only
that path was insufficient. The test covers the small example and an opt-in
version with a TARGET-sized first chunk. The unpatched implementation was
observed to fail the small reproduction.

Validation: both cases pass on this branch with the existing public Corgi pin:

CARGO_PROFILE_RELEASE_DEBUG=0 cargo test --locked --offline --release \
  -p interactive --test corgi_chunk_merge -- --include-ignored

This fixes a sorted/consolidated merge-contract violation. It does not claim
to explain an LDBC performance result or a particular wrong query answer.

@frankmcsherry
frankmcsherry merged commit d77d8a7 into master-next Sep 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant