Conversation
| /** | ||
| * When receiving a zero-copy stream, validate Digest.crc32 before the sstable is made visible. | ||
| */ | ||
| public volatile boolean entire_sstable_stream_digest_validation_enabled = false; |
There was a problem hiding this comment.
In 4.0, 4.1, and 5.0, I could see leaving this false by default to preserve behavior on minor upgrade. In 6.0+ I think we want it to be true by default, with an accompanying note in the NEWS.txt "Upgrading" section for 6.0.
WDYT?
| try | ||
| { | ||
| new DataIntegrityMetadata.FileDigestValidator(descriptor.fileFor(Components.DATA), | ||
| descriptor.fileFor(Components.DIGEST)).validate(); |
There was a problem hiding this comment.
validate() throws a NumberFormatException if the digest is malformed. Do we want that to count against countEntireSSTableDigestMismatch() or just flow through?
There was a problem hiding this comment.
Good catch, it makes more sense to count it toward countEntireSSTableDigestMismatch since it technically still is one. But I notice the other place that validates digests (SortedTableVerifier.verifyDigest()) doesn't catch a NumberFormatException, only IOException. The more reliable fix seems to be throwing an IOException, not a NumberFormatException, so it's correctly caught and handled in both sites. Updated with this change!
2 participants
This patch adds CRC validation on the receiving side, after an entire sstable is received via zero-copy streaming. This detects SSTables that were already corrupt on the sender's disk and prevents them from being accepted, which stops the corruption from spreading.
Since it's possible for the digest component not to be streamed, this check is disabled by default in the config via
entire_sstable_stream_digest_validation_enabled, and fails open (if no digest is present, the streaming succeeds).Validation costs one sequential read of the data file per received SSTable, and any mismatch makes the streaming session fail and increments a metric for
entireSSTableDigestMismatches.The Cassandra Jira