Skip to content

fix(storage-cassandra): index rows follow the base item under every write path - #424

Open
LeeroyHannigan wants to merge 1 commit into
mainfrom
fix/cassandra-index-row-ordering
Open

LeeroyHannigan wants to merge 1 commit into
mainfrom
fix/cassandra-index-row-ordering

Conversation

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator

Closes #414.

What was wrong

On Cassandra, every write commits the base row with a lightweight transaction and then writes the item's secondary-index rows in a separate logged batch, with the batch timestamps left to the coordinator. Measured against Amazon DynamoDB and reproduced on a main server, that produced four defects:

  1. The race in storage-cassandra: a PutItem that loses the create race can leave its LSI row behind #414. Two writers to one item commit their base rows in Paxos order, but their index batches can reach the index table in the other order, and the later version's delete of the earlier version's row is undone when the earlier batch lands. 14 of 40 delete-versus-put rounds on an LSI table left the index disagreeing with the base table; the service left none.
  2. Any write that keeps the index key removed the item from every index: UpdateItem on a non-key attribute, PutItem with the same keys, and a transactional Update each emitted a DELETE and an INSERT of one index row in one batch, both at the batch timestamp, and the tombstone won. Every such write did this on both LSIs and GSIs.
  3. An item created by TransactWriteItems could not be updated, overwritten or conditionally deleted afterwards. The commit's version = version + 1 left a PREPARE-created row's version null, and the ordinary write paths fenced a null as 0.
  4. A transactional Delete left the item's index rows in place, and a transactional Put over an existing item left the rows of the image it replaced.

What changes

crates/storage-cassandra only.

  • version becomes max(previous + 1, now) in microseconds since the epoch, bound as a value in every statement that stores it. It is still the OCC token; a row whose version is null fences on IF version = null.
  • Every index batch a base-row mutation produces is stamped with that version: the post-LWT batch of put_item and update_item, the secondary-effects batch of delete_item, the transaction commit's batch, and the gsi_pending entry an asynchronous GSI queues (the worker applies the stamp). A later version's index mutation beats an earlier version's whatever order the batches arrive in.
  • An index row whose primary key is unchanged between the old and new image is rewritten by the INSERT alone (same_index_row), in the sync path and in the GSI worker.
  • Ordinary deletes without TTL go through the IF version = ? fence and retry a lost race from a fresh read; deleting an absent item writes nothing.
  • The transaction commit reads the existing image and version under its PREPARE fence, binds a concrete version, and syncs index rows for Put, Update and Delete. COMMITTING recovery syncs index rows from the ledger's two images, only for rows whose commit fence it applied itself, stamped with the version it stored.

docs/adr/0022-cassandra-index-row-version-stamps.md records the decision and its consequences; ADR-0020 is amended where it described the version value and the commit statement.

Verification

  • tests/test_index_rows_track_base_item.py: 11 dual-target cases (writes that keep the index key; transactional Put, Update and Delete; ordinary writes on a transaction-created item; a delete racing a put over 25 rounds). 11 of 11 pass on the service and on Cassandra; 9 of 11 fail on main.
  • New direct integration tests: the same-row rule emits one statement and keeps the row; version stamps order index mutations under reversed arrival; the async worker keeps the row across a same-key update; every write path stamps its index row with the stored version; COMMITTING recovery leaves a row a later writer owns alone and repairs a row it commits. 139 direct and 27 TTL integration tests pass.
  • Negative controls, each failing its test with the guard removed: the same-row rule in the sync path and in the worker, the strictly increasing version, the batch stamp at occ_write, at the IF NOT EXISTS create and at the transaction commit, and the recovery gate.
  • Regression against a Cassandra server built from this branch: 1,053 passed in the backend-agnostic suite. Every failure was in a suite devtools/run-tests runs separately (tests/python, the CLI lifecycle files, the PostgreSQL-only GSI queue file).
  • fmt, clippy 1.97 -D warnings, MSRV 1.88 and the crate's 58 unit tests are clean. No dependency change, so the license notices are unchanged.

Consequences

  • An unconditional delete on a table without TTL now costs a Paxos round.
  • Rows written before this change carry a counter or a null version and move into the microsecond domain on their next write. Index rows written before this change carry coordinator timestamps, which the next write's stamp exceeds.
  • During a rolling upgrade, a node still on the old binary accepts the new versions but keeps dropping the index rows of the items it writes (defect 2); the next write from an upgraded node restores them.
  • Two cases keep the old clock ordering and are recorded in the ADR: a create after a delete is ordered against the tombstones by the wall clock (the deleted row leaves no version), and the TTL-claimed paths keep their pinned wall-clock batch timestamp because the claim cells must not land above a later claim's Paxos timestamp.
  • The asynchronous GSI workers still stop at startup under serve (storage-cassandra: asynchronous GSI writes are never applied under serve (default 10 ms delay) #407); the worker path here is exercised by the crate's direct integration tests, which hold the worker guard.
  • The sort-key and hash-only delete arms carry the same fence logic twice, and the stamp is threaded as a positional argument through the index helpers; both are candidates for a follow-up refactor and are left as they are here.

…rite path

Closes #414.

Every Cassandra write commits the base row with a lightweight transaction and
writes the item's secondary-index rows in a separate logged batch. The batch
timestamps were left to the coordinator, which produced four defects, each
measured against Amazon DynamoDB and reproduced on a main server:

1. Two writers to one item commit base rows in Paxos order but their index
   batches arrive in the other order, and the later version's delete of the
   earlier version's index row is undone when the earlier batch lands. 14 of
   40 delete-versus-put rounds on an LSI table left the index disagreeing with
   the base table; the service left none. This is the race issue #414 reports.
2. A write that keeps the index key (UpdateItem on a non-key attribute, PutItem
   with the same keys, transactional Update) emitted a DELETE and an INSERT of
   one index row in one batch. Both carried the batch timestamp, the tombstone
   won, and the item left every index. Every such write did this.
3. An item created by TransactWriteItems could not be updated, overwritten or
   conditionally deleted afterwards: the commit's `version = version + 1` left
   a PREPARE-created row's version null, and the ordinary write paths fenced a
   null as 0.
4. A transactional Delete left the item's index rows in place; a transactional
   Put over an existing item left the rows of the image it replaced.

The `version` column becomes max(previous + 1, now) in microseconds since the
epoch, bound as a value in every statement that stores it, and every index
batch a base-row mutation produces is stamped with it: the post-LWT batch of
put_item and update_item, the secondary-effects batch of delete_item, the
transaction commit's index batch, and the gsi_pending entry an asynchronous GSI
queues, whose context carries the stamp for the worker. A later version's index
mutation therefore beats an earlier version's whatever order the batches arrive
in. An index row whose primary key is unchanged between the old and new image
is rewritten by the INSERT alone. Ordinary deletes without TTL go through the
same `IF version = ?` fence as writes and retry a lost race from a fresh read.
The transaction commit reads the existing image and version under its PREPARE
fence, binds a concrete version, and syncs index rows for Put, Update and
Delete. Recovery of a COMMITTING transaction syncs index rows from the ledger's
two images, only for rows whose commit fence it applied itself, stamped with
the version it stored. A row whose version is null fences on `IF version =
null`. No storage backend other than Cassandra changes.

Verification:

- tests/test_index_rows_track_base_item.py: 11 dual-target cases covering
  writes that keep the index key, transactional Put, Update and Delete,
  ordinary writes on a transaction-created item, and a delete racing a put
  over 25 rounds. 11 of 11 pass on the service and on Cassandra; 9 of 11 fail
  on main.
- Direct integration tests: the same-row rule emits one statement and keeps
  the row; version stamps order index mutations under reversed arrival; the
  async worker keeps the row across a same-key update; every write path stamps
  its index row with the stored version; COMMITTING recovery leaves a row a
  later writer owns alone and repairs a row it commits. 139 direct and 27 TTL
  integration tests pass.
- Negative controls, each failing its test with the guard removed: same-row
  rule (sync path and worker), strictly increasing version, the batch stamp at
  occ_write, at the IF NOT EXISTS create and at the transaction commit, the
  recovery gate.
- Regression against a Cassandra server built from this commit: 1,053 passed
  in the backend-agnostic suite; every failure is in the suites run-tests
  runs separately (tests/python, the CLI lifecycle files, the PostgreSQL-only
  GSI queue file).
- Six unit tests on the version rule. fmt, clippy 1.97 -D warnings, MSRV 1.88,
  and the crate's 58 unit tests are clean. No dependency change.

Docs: ADR-0022 records the decision and its consequences (an unconditional
delete now costs a Paxos round; rows written before this change move into the
microsecond domain on their next write; during a rolling upgrade a node on the
old binary still drops the index rows of items it writes until an upgraded
node writes them again; the asynchronous GSI workers still stop at startup
under serve, issue #407). ADR-0020 is amended where it described the version
value and the commit statement.

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.

storage-cassandra: a PutItem that loses the create race can leave its LSI row behind

1 participant