cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jason Brown (JIRA)" <>
Subject [jira] [Commented] (CASSANDRA-13321) Add a checksum component for the sstable metadata (-Statistics.db) file
Date Tue, 03 Oct 2017 15:44:00 GMT


Jason Brown commented on CASSANDRA-13321:

Coming back to this after a long time I agree with the decision to simplify the solution for
adding checksums, and the most recent branch satisfies that. 

A few small comments:

- on the serialize path, call {{DataOutputBuffer@getData()}} instead of {{DataOutputBuffer#toByteArray}}
as the latter allocates a new buffer and copies, whereas the former just hands over it's backing
byte array from the {{ByteBuffer}}.
- {{Hashing.md5()}} - we *could* choose to swap to some other, more lighter weight algo from
guava'a {{Hasher}}, but as this code path is called very infrequently it's probably not worth
- on the deserialize path, you build up the {{lengths}} map in the first {{for}} loop. Then
in the second {{for}} loop, you determine the {{size}} to read from the {{in}} stream. Admittedly,
it took me some staring at that {{if}} to figure out what exactly it was doing. While correct,
it might be friendlier for code reading if we add the length for the {{lastType}} to the map
after the first {{for}} loop completes - then you won't need the {{if}} branching in the second

Beyond these nits, I'm +1. Nice work simplifying this patch to the minimal work required.

> Add a checksum component for the sstable metadata (-Statistics.db) file
> -----------------------------------------------------------------------
>                 Key: CASSANDRA-13321
>                 URL:
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Marcus Eriksson
>            Assignee: Marcus Eriksson
>             Fix For: 4.x
> Since we keep important information in the sstable metadata file now, we should add a
checksum component for it. One danger being if a bit gets flipped in repairedAt we could consider
the sstable repaired when it is not.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message