Repository navigation
perf(storage-cassandra): restore copy writes plain INSERTs, not the LWT path - #527
Open
LeeroyHannigan wants to merge 1 commit into
Open
LeeroyHannigan wants to merge 1 commit into
LeeroyHannigan wants to merge 1 commit into
Conversation
…WT put path RestoreTableFromBackup copied each backup item through put_item_impl: a read of the (empty) target row, a lightweight transaction, and a logged batch for the index rows, eight items at a time. The target is CREATING for the whole copy and the copy is its only writer, so none of that guards anything. data/restore_copy.rs holds the copy's reads and writes. read_backup_bucket returns one payload bucket's items. restore_writer resolves the target's indexes once per copy (LOCAL_QUORUM), and write_restored_item writes an item as plain INSERTs with no read and no IF condition: the base row (pk, sort key column, item_data, version) and one row per GSI and LSI the item carries keys for, issued concurrently at LOCAL_QUORUM. Index rows come from index_row_insert, so their shape and projection are the put path's. The index key type and emptiness validation is kept. version is the clock reading in microseconds, the value a later IF version = ? fence compares against. Neither the base row nor the index rows carry a client timestamp: every later index mutation on this tree (put, update, delete, transaction commit, the GSI queue worker) is stamped by the coordinator, and a restored row stamped from the host clock that ran the restore could outlive a later delete whose coordinator clock trailed that host. Nothing else is written: a restored table starts with streams disabled, TTL settings are not restored, and every GSI row is written in the copy, so there is no gsi_pending entry. copy_backup_into_target reads payload buckets two ahead of the writes and writes 64 items at a time across bucket boundaries, checking the target's ownership per item as well as per bucket, so a swept target stops taking writes within one item. The size accumulation and the item count check are unchanged. The IndexWrites enum and put_restored_item that the old copy needed are removed from put_get_item.rs. Measured on one host against a single cassandra:4.1 node with a 4 GB heap, from the CREATING response to ACTIVE: 60,000 items with no index 27.8 s before, 2.2 s after; 20,000 items with one GSI and one LSI 33.9 s before, 0.7 s after. Item counts, every item and every index row compared equal between source and target in both shapes. Direct tests: restored rows take every later write (update, conditional put and delete, transactions, across GSI and LSI rows); the index rows are coordinator-stamped; a backup item with a mistyped index key fails the restore; a recorded item count that disagrees with the payload fails the restore; the restored row shape equals the put path's. Four existing tests that probe the CREATING window use 10,000 items so the copy outlasts their probes, and the simultaneous-restore test holds a registration on the backup so exclusivity is asserted against a held registration rather than a race. Closes #404
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
October 24, 2026 21:57
This was referenced Oct 24, 2026
This branch has not been deployed
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.
Summary
RestoreTableFromBackupon Cassandra copied each backup item through the ordinary put path: a read of the empty target row, a lightweight transaction, and a logged batch for the index rows, eight items at a time (#404). The target isCREATINGfor the whole copy and the copy is its only writer, so none of that guards anything. A 60,000-item restore took 27.8 s on this host against 1.5 to 3.6 s on the other three backends.The copy now writes each item as plain
INSERTs with no read and noIFcondition: the base row and one row per GSI and LSI the item carries keys for, built by the same function the put path uses so their shape and projection are identical, 64 items in flight with two payload buckets read ahead. Measured from theCREATINGresponse toACTIVE: 60,000 items with no index 27.8 s to 2.2 s; 20,000 items with one GSI and one LSI 33.9 s to 0.7 s. In both shapes the item count, every item (Scan) and every index row (Queryon each index partition) compared equal between source and target.Stamps on restored rows
Every later conditional write fences on the row's
version, so restored rows carry one: the clock reading in microseconds, which is the value a laterIF version = ?compares against on this tree and the value #424 moves new rows to. Neither the base row nor the index rows carry a client write timestamp: every later index mutation on this tree (put, update, delete, transaction commit, the GSI queue worker) is stamped by the coordinator, and a restored index row stamped from the host clock that ran the restore could outlive a later delete whose coordinator clock trailed that host (the Cassandra rule, reproduced with cqlsh: a cell stamped ahead of the coordinator survives an unstamped delete). Once index mutations carry version-derived timestamps (#424), the restored index rows takeversionas theirs; that is a one-argument follow-up noted inrestore_copy.rs.Direct tests pin the behaviour: after a restore,
UpdateItemmoving an item across GSI and LSI partitions, conditionalPutItemandDeleteItemon restored values,attribute_not_existson a restored key, andTransactWriteItemsover restored items all behave as on put-path rows; the restored row shape (SELECT JSON *of base and index tables, minusversion) equals the put path's; the index rows are coordinator-stamped.Recovery
The copy runs in the task #384 detached from the request, with its heartbeat. Measured on the node: a target data table dropped from outside mid-copy was detected and the target removed from the catalog in 1.2 s, after which a second restore of the same backup and a
CreateTableof the name both succeeded; a node restart mid-copy left the targetCREATINGfor the sweep, which the next server start ran (the client session does not reconnect after a node restart on this tree, so the in-process sweep could not; that is #403 and #457's territory, noted there). The copy checks the target's ownership per item as well as per bucket, so a swept target stops taking writes within one item. Two restores at once complete normally; a 60,000-item restore under a sixteen-threadPutItemstorm on another table took 10.3 s with zero writer errors and no node warnings. 64 writes in flight is far below one connection's 32,767 stream ids.Tests
crates/storage-cassandra/tests/direct/restore_write.rs(2): restored rows take every later write; pk-only rows across two payload buckets.crates/storage-cassandra/tests/direct/restore_write_controls.rs(4): coordinator-stamped index rows; a backup item with a mistyped GSI key fails the restore (the target is removed, neverACTIVE); a recorded item count that disagrees with the payload fails the restore; the restored row shape equals the put path's. Each fails under the mutant it was written for (host-clock stamp;validate_index_keysremoved; count check removed; an extra column written).crates/storage-cassandra/tests/direct/restore_timing.rs(2, ignored): the two measured shapes, run with--ignored.CREATINGwindow use 10,000 items so the copy outlasts their probes; the simultaneous-restore test holds a registration on the backup and asserts every concurrent restore isBackupInUsenaming the held target, then that the next restore is admitted once the registration is released (10 of 10 runs); its sequential section is unchanged.backup_engine::direct tests 16,--lib64; the backup and restore wire suite 36 passed on a Cassandra server;cargo fmt,clippy1.97.0 with-D warnings, 1.88.0 check: clean. No dependency change. New files under 600 lines;data/index.rs723 to 748,tests/direct/backup_engine.rs2091 to 2155 (pre-existing files over 600).Merge notes
fix/cassandra-index-row-ordering): the two conflictsmainalready has with it (tests/direct/index.rs,docs/adr/README.md);put_get_item.rsmerges because this branch returns it to its pre-fix(backup): restore sort keys, indexes, and throughput; bound memory; recover abandoned restores #384 text. After fix(storage-cassandra): index rows follow the base item under every write path #424 lands,restore_copy.rstakesnext_row_version(None)forversionandSome(version)as the index rows' timestamp.fix/issue-403-cassandra-connection-resilience), Store U+0000 in strings on PostgreSQL and MongoDB #368, feat(core): backup format v1: streaming data files, sidecar manifest hash, typed backup ARN [backup/restore 3/4] #394, feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395: clean.Not in this PR
Pre-existing, noted or filed separately: the Cassandra client session does not reconnect after a node restart, so a restore that fails while the node is unreachable cannot abandon its target and the in-process sweep does not run until the server restarts (#403, #457);
CreateBackupon Cassandra is bound to the request future (a dropped 200,000-item request left no backup record); the restore response includes the target's DDL (10.7 s on a node with 470 tables);TransactWriteItemsPut over an existing item leaves its old index row when the put moves an index key (#424 fixes it);test_restore_sweep_worker_sweeps_at_startup_and_stops_on_shutdownhas an 8 s budget that a loaded host can exceed.Closes #404