Skip to content

store: reject write batches that overlap the deployment head - #6755

Open
lx3133584 wants to merge 1 commit into
graphprotocol:masterfrom
lx3133584:store/reject-overlapping-batches
Open

lx3133584 wants to merge 1 commit into
graphprotocol:masterfrom
lx3133584:store/reject-overlapping-batches

Conversation

@lx3133584

Copy link
Copy Markdown

What

deployment::transact_block compared only the last block of a write batch with the deployment head. A batch covers a range, and the head can advance into the middle of that range while the batch is being prepared.

When a deployment is reassigned, the old runner commits its pending batch while stopping, and the new runner — which started from the same head — later commits a batch covering blocks the old runner already wrote. Both batches were accepted. Every entity in the overlap then got a second version that was also live at the head, so list queries returned it twice.

This is what #6745 reports: two runners on one deployment, 12 / 6000 deployments affected in production, and the resulting attestation disputes.

The fix compares the batch's first block instead of its last one. A batch whose range is already covered by the head has lost the race and is refused with DuplicateBlockProcessing — the same error a single duplicate block already produced, so existing handling is unchanged.

Batch::first_block already existed and is preserved when batches are appended, so no new plumbing was needed beyond passing it through the one call site.

Why the check is at the head, not in the constraint

For spec 1.3.0 deployments the vid primary key (block << 32 | seq) already rejects an overlapping batch, because both writers assign the same vids. For earlier specs vid is bigserial, every writer gets fresh vids, and nothing at the database level objects — which is why the affected deployments are the older ones. The head comparison covers both.

Tests

store/test-store/tests/postgres/overlapping_batches.rs builds two independent Store instances against one database, so the per-process writables cache and the in-memory check in WritableStore cannot see each other — the test reproduces two graph-node processes.

  • overlapping_batches_from_two_writers_are_rejected — A commits blocks 1..3, B commits 1..5. Fails on master: B's flush succeeds, the table holds two live versions of the entity, and the list query at head returns it twice. Passes with the fix, refused with DuplicateBlockProcessing(deployment, 1).
  • spec_1_3_overlapping_batch_hits_vid_pk — control for the spec 1.3.0 case, where the vid primary key fires first.
  • single_writer_leaves_one_live_version — control, one writer on the same range.

Tested against PostgreSQL 14:

cargo test -p test-store --test postgres
test result: ok. 101 passed; 0 failed; 0 ignored

Fixes #6745

transact_block compared only the last block of a batch with the deployment
head. A batch covers a range, and the head can advance into the middle of that
range while the batch is being prepared: when a deployment is reassigned, the
old runner commits its pending batch while stopping, and the new runner, which
started from the same head, later commits a batch covering blocks the old
runner already wrote. Both were accepted, so every entity in the overlap got a
second version that was also live, and list queries returned it twice.

Compare the batch's first block instead of its last one. A batch whose range is
already covered by the head has lost the race and is refused with
DuplicateBlockProcessing, the same error a single duplicate block already
produced.

The new test drives two Store instances against one database, so the
in-process caches cannot hide the second writer. It fails on master with two
live versions of the entity and the list query returning it twice.

Signed-off-by: 梁旭 <liangxu.5@bytedance.com>

This branch has not been deployed

No deployments
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.

Store: overlapping write batches from two runners are accepted, leaving duplicate entity versions (head check only looks at the last block of a batch)

1 participant