Avoid Rebuilding Per-SSTable SAI components - #4962
Conversation
| assertNotEquals(snapshotLastModified, indexFilesLastModified()); | ||
| assertNumRows(2, "SELECT * FROM %%s WHERE v1 >= 0"); | ||
| assertValidationCount(2, 2); // compaction should not validate | ||
| // CASSANDRA-21515: a single-index rebuild no longer rewrites the shared per-SSTable components |
There was a problem hiding this comment.
I would reword to indicate something like this that indicates exactly why we see 4:
per-SSTable components are validated both during rebuild and in
verifyIndexFiles()
maedhroz
left a comment
maedhroz
left a comment
There was a problem hiding this comment.
Just a few little things to fix, but otherwise looks pretty good.
dcapwell
left a comment
dcapwell
left a comment
There was a problem hiding this comment.
Code Looks good to me, but put this through a AI review and got some valid feedback:
Issue
[testing/Missing test for a newly added error path] shouldWritePerSSTableFiles() dropped the '|| isFullRebuild' term, so a manual full rebuild now regenerates the shared per-SSTable components ONLY when !isPerSSTableIndexBuildComplete() or !validatePerSSTableComponents(CHECKSUM). The corruption-recovery path (a full rebuild must still regenerate corrupt shared components) is now solely load-bearing on that validation. No test in this change, and no existing test, exercises a manual full rebuild (isFullRebuild=true) against a table whose shared per-SSTable component is corrupt: verifyRebuildCorruptedFiles/verifyRebuildIndexComponent explicitly 'return' for PER_SSTABLE_COMPONENTS (StorageAttachedIndexDDLTest.java:919), and NodeStartupTest's PER_SSTABLE_CORRUPT scenario covers only the node-startup build path, not rebuildIndexes(). — The change removed the unconditional rewrite on full rebuild and made checksum validation the sole trigger for regenerating corrupt shared components during a manual rebuild. A regression in that validation (or in the delete/rewrite branch) on the full-rebuild path would silently leave corrupt shared components in place and no test would catch it. The happy path (rebuild skips rewrite) is well covered, but this newly load-bearing error path is not.
Action
Add a test that corrupts a shared per-SSTable component (e.g. GROUP_META / ROW_TO_PARTITION), performs a manual full rebuild via rebuildIndexes(...), and asserts the shared components are regenerated (SSTableContext.create is invoked) and both indexes remain queryable. This closes the corner the change now depends on.
3 participants
Cassandra-21515
rebuild index on a single SAI index no longer rewrites the shared per-SSTable components.
patch by Sunil Ramchandra Pawar;