[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14140015#comment-14140015 ] Jason Brown commented on CASSANDRA-7978: +1. I guess you're getting bit by it right now, then , huh? :) I'm working in the compression part of the code and could work on this if you're busy running upgradesstables > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Priority: Minor > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14140876#comment-14140876 ] sankalp kohli commented on CASSANDRA-7978: -- Yes please. Assigned to you. Also can you knock off CASSANDRA-7928 . It is very hard to pass the value down and I don't want to use thread locals as it will be a dirty hack :) > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: Jason Brown >Priority: Minor > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14140890#comment-14140890 ] Jason Brown commented on CASSANDRA-7978: bq. Also can you knock off CASSANDRA-7928 Would like fries would that, sir? :) > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: Jason Brown >Priority: Minor > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14140903#comment-14140903 ] sankalp kohli commented on CASSANDRA-7978: -- [~jasobrown] Well you think it is possible to pass that value down to the storage layerso be my guest :) > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: Jason Brown >Priority: Minor > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14140910#comment-14140910 ] Jason Brown commented on CASSANDRA-7978: Actually, I don't, at least, not without making a horrifying mess of the internal APIs - that no one, including me, would ever want to commit. But let's leave that discussion out of this ticket. > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: Jason Brown >Priority: Minor > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14141045#comment-14141045 ] Jason Brown commented on CASSANDRA-7978: The patch allows us to set the value when the sstable is opened, but then it's 'fixed' for that sstable for the life of the process, correct? I think we'd want to be able to change the value live and have it immediately reflected. Also, as a minor point (probably not worth bothering with), should we stop stop writing the crc_check_chance into the sstable, as well? > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14141178#comment-14141178 ] T Jake Luciani commented on CASSANDRA-7978: --- bq. I think we'd want to be able to change the value live and have it immediately reflected. This isn't the kind of setting you want to constantly tune live so I'm not sure we want to get too fancy with it (rolling restart seem reasonable) and keep in mind things would improve over time due to compaction. If you feel strongly we could add code to reopen all the readers when this setting is changed. > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14141244#comment-14141244 ] Jason Brown commented on CASSANDRA-7978: >> bq. rolling restart seem reasonable I don't completely agree with this. In some situations/organizations, rolling restarts are not as willy nilly as one might wish. Further, what if you have 1000s of nodes in the cluster? Rolling restart might take a day or two, depending on your tolerance for inflicting pain on users :). bq. we could add code to reopen all the readers when this setting is changed I don't think we need to go far, but maybe just visit all the open readers and update the local copy of the compression settings there should be sufficient, perhaps? > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14141322#comment-14141322 ] Benedict commented on CASSANDRA-7978: - We only need to set the value wherever it's retrieved from, and call FileCacheService.invalidate() on each of the files we've changed the setting for. I'd kind of prefer, if we want to make this setting configurable live, to drop maintenance of the setting from compression metadata and just fetch it from the CFMetaData each time we open a reader. > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14143741#comment-14143741 ] Jason Brown commented on CASSANDRA-7978: Reviewed v2, and I wonder if it's better to update the CP.crcCheckChance as it's a non-final field (and volatile, at that) rather than retain a reference to the CFMD. > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt, 7978v2.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14143747#comment-14143747 ] T Jake Luciani commented on CASSANDRA-7978: --- When you merge the CFMD it replaces the entire CP so it needed to be higher level then CP. > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt, 7978v2.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14143781#comment-14143781 ] Jason Brown commented on CASSANDRA-7978: I just found this ticket, #CASSANDRA-5053, which exposes CFS.setCrcCheckChance() as an mbean value (configurable at runtime). Would that be sufficient for [~kohlisankalp]'s needs, of being able to adjust the CRC check chance dynamically (without restart/upgradesstables/etc)? > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt, 7978v2.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14143802#comment-14143802 ] T Jake Luciani commented on CASSANDRA-7978: --- I thought doing things on 1000's of nodes was a pain :) also that is an ephemeral change. The v2 patch doesn't require any restart/upgradesstables/etc, it updates as soon as the change propagates. > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt, 7978v2.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14143893#comment-14143893 ] Jason Brown commented on CASSANDRA-7978: bq. that is an ephemeral change d'oh! yes, you are correct. Then, I'm +1 on the patch. The only nit I would have (and it's rather trivial) is to perhaps store a reference to an "override" CompressionParameters rather than the whole CFMD (as live endpoints) in the updated CompressionParameters. [~benedict] the reason why FileCacheService.invalidate() wouldn't work is that FCS holds RAR/CRAR instances, and CRAR gets it's CompressionMetadata from a CompressedPoolingSegmentedFile wrapper (derived from SegmentedFile), and CPSF what SSTR holds onto for it's lifetime. Thus, CPSF never gets 'updated' or recycled (via FCS) in the way that CRAR does, so calling FileCacheService.invalidate() won't do the trick. > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt, 7978v2.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14144196#comment-14144196 ] T Jake Luciani commented on CASSANDRA-7978: --- I would need to create a wrapper class to hold the compression parameters otherwise the compressed reader won't see the live updates. That is exactly what CFMD is doing here. I don't see a reason to not use it. > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt, 7978v2.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-7978) CrcCheckChance should not be stored as part of an sstable
[ https://issues.apache.org/jira/browse/CASSANDRA-7978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14144324#comment-14144324 ] Jason Brown commented on CASSANDRA-7978: I was slightly concerned about the unexpected appearance of the CFMD that's only used in one place, and hoped we could maybe get around that (as well as reducing the scope of CP's dependencies). You are correct that we need some wrapper to get those updates, and I just now spent time trying to see if it's just easier to pass the CFMD instance into the CP constructor (and always have a reference to that CFMD, rather than waiting for SSTR.getCompressionMetadata()). Unfortunately, it's not simple as CP has 4 constructors, called from different paths, and being able to pass the correct CFMD instance to those is not overly friendly. So, I'll stop making noise now and let you commit the patch so we can move on :). > CrcCheckChance should not be stored as part of an sstable > - > > Key: CASSANDRA-7978 > URL: https://issues.apache.org/jira/browse/CASSANDRA-7978 > Project: Cassandra > Issue Type: Improvement >Reporter: sankalp kohli >Assignee: T Jake Luciani >Priority: Minor > Fix For: 2.0.11 > > Attachments: 7978.txt, 7978v2.txt > > > CrcCheckChance is stored with compression parameters in the sstable. The only > way to change it is to do upgrade sstable. I don't see why it should not be a > hot property. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
