[jira] [Comment Edited] (CASSANDRA-13321) Add a checksum component for the sstable metadata (-Statistics.db) file
[ https://issues.apache.org/jira/browse/CASSANDRA-13321?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15956443#comment-15956443 ] Marcus Eriksson edited comment on CASSANDRA-13321 at 4/5/17 7:42 AM: - I made an alternative implementation of this: https://github.com/krummas/cassandra/commits/marcuse/appendedchecksum which just appends the checksum within the same file, much simpler but does not solve all the issues that the versioned component patch above solves, but feels much safer. We still need to rename the file on top of the old file using this patch, so not sure if the risk of the old patch is worth it, wdyt [~jasobrown]? Also note that we don't read the whole -Statistics.db file, instead we read the individual components within the file (validation metadata, compaction metadata etc), so each component has its own checksum in this patch was (Author: krummas): I made an alternative implementation of this: https://github.com/krummas/cassandra/commits/marcuse/appendedchecksum which just appends the checksum within the same file, much simpler but does not solve all the issues that the versioned component patch above solves, but feels much safer. We still need to rename the file on top of the old file using this patch, so not sure if the risk of the old patch is worth it, wdyt [~jasobrown]? > Add a checksum component for the sstable metadata (-Statistics.db) file > --- > > Key: CASSANDRA-13321 > URL: https://issues.apache.org/jira/browse/CASSANDRA-13321 > 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 (v6.3.15#6346)
[jira] [Comment Edited] (CASSANDRA-13321) Add a checksum component for the sstable metadata (-Statistics.db) file
[ https://issues.apache.org/jira/browse/CASSANDRA-13321?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16185878#comment-16185878 ] Marcus Eriksson edited comment on CASSANDRA-13321 at 9/29/17 2:51 PM: -- I have rebased and fixed a few issues with the simpler solution (https://github.com/krummas/cassandra/commits/marcuse/simplerchecksum) - I think we should go with that version for now as the swapping of sstable metadata seems to be working (and this doesn't change that part at all) https://circleci.com/gh/krummas/cassandra/126 https://builds.apache.org/view/A-D/view/Cassandra/job/Cassandra-devbranch-dtest/351/ -Note that the checksumming is a bit awkward since I didn't want to add a length before each metadata component, instead everything is hashed as it is being read. With sstable metadata, we could probably do that and verify the actual bytes instead - this is something you could consider during review- edit: checksumming the bytes instead was much nicer, so doing that, link to tree is updated in this comment was (Author: krummas): I have rebased and fixed a few issues with the simpler solution (https://github.com/krummas/cassandra/commits/marcuse/appendedchecksum) - I think we should go with that version for now as the swapping of sstable metadata seems to be working (and this doesn't change that part at all) https://circleci.com/gh/krummas/cassandra/125 https://builds.apache.org/view/A-D/view/Cassandra/job/Cassandra-devbranch-dtest/350/ Note that the checksumming is a bit awkward since I didn't want to add a length before each metadata component, instead everything is hashed as it is being read. With sstable metadata, we could probably do that and verify the actual bytes instead - this is something you could consider during review > Add a checksum component for the sstable metadata (-Statistics.db) file > --- > > Key: CASSANDRA-13321 > URL: https://issues.apache.org/jira/browse/CASSANDRA-13321 > 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 (v6.4.14#64029) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-13321) Add a checksum component for the sstable metadata (-Statistics.db) file
[ https://issues.apache.org/jira/browse/CASSANDRA-13321?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16190846#comment-16190846 ] Marcus Eriksson edited comment on CASSANDRA-13321 at 10/4/17 5:43 AM: -- thanks for the review, fixed the comments [here|https://github.com/krummas/cassandra/commits/marcuse/simplerchecksum] and rerunning tests [here|https://builds.apache.org/view/A-D/view/Cassandra/job/Cassandra-devbranch-dtest/359/] before committing was (Author: krummas): thanks for the review - rerunning tests [here|https://builds.apache.org/view/A-D/view/Cassandra/job/Cassandra-devbranch-dtest/359/] before committing > Add a checksum component for the sstable metadata (-Statistics.db) file > --- > > Key: CASSANDRA-13321 > URL: https://issues.apache.org/jira/browse/CASSANDRA-13321 > 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 (v6.4.14#64029) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
