[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13592395#comment-13592395 ] Jonathan Ellis commented on CASSANDRA-4795: --- bq. I'm fine with those To clarify, is that I'm fine in principle or I've reviewed it, +1? replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 0001-Reallow-unexpected-strategy-options-for-thrift.txt, 0002-Reallow-unexpected-strategy-options-for-thrift.txt, 0003-Adds-application_metadata-field-to-ks-metadata.txt, 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13592621#comment-13592621 ] Pavel Yaskevich commented on CASSANDRA-4795: I've reviewed those two, +1. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 0001-Reallow-unexpected-strategy-options-for-thrift.txt, 0002-Reallow-unexpected-strategy-options-for-thrift.txt, 0003-Adds-application_metadata-field-to-ks-metadata.txt, 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13590518#comment-13590518 ] Sylvain Lebresne commented on CASSANDRA-4795: - So, does the absence of review of my 2 first patches mean that everyone decided the status quo was ok? Cause I could live with that. But if we think this should be reverted from 1.2 then let's do that sooner than later. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 0001-Reallow-unexpected-strategy-options-for-thrift.txt, 0002-Reallow-unexpected-strategy-options-for-thrift.txt, 0003-Adds-application_metadata-field-to-ks-metadata.txt, 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13590852#comment-13590852 ] Pavel Yaskevich commented on CASSANDRA-4795: bq. Show me an existing database that lets you attach random crap to a table definition and I will reconsider my position. What is HBase? bq. So, does the absence of review of my 2 first patches mean that everyone decided the status quo was ok? Cause I could live with that. But if we think this should be reverted from 1.2 then let's do that sooner than later. I'm fine with those, I'm fine even with removing those as soon as system doesn't crap on startup. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 0001-Reallow-unexpected-strategy-options-for-thrift.txt, 0002-Reallow-unexpected-strategy-options-for-thrift.txt, 0003-Adds-application_metadata-field-to-ks-metadata.txt, 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13577397#comment-13577397 ] Sylvain Lebresne commented on CASSANDRA-4795: - Do we agree on the first 2 patches in the meantime? replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 0001-Reallow-unexpected-strategy-options-for-thrift.txt, 0002-Reallow-unexpected-strategy-options-for-thrift.txt, 0003-Adds-application_metadata-field-to-ks-metadata.txt, 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13576449#comment-13576449 ] Sylvain Lebresne commented on CASSANDRA-4795: - bq. If the original intention was to protect people from mistypes How can that *not* be one's intent? bq. like git does e.g. option rplicatian_factor is not recognized maybe you mean 'replication_factor'? Sure, that would be great: reject errors with fix suggestion in the error message is indeed even better than reject errors that is itself better than not reject errors silently. So be my guest, do open a ticket to do even better than what this patch does. But that patch is still progress. bq. it's pretty convenient to keep it in the keypsace strategy options (without have any other alternatives) Again, I'm ok talking about alternatives that would be right if there is a need, but you will not convince me that shoving random application data inside the strategy options is right, or even a good idea. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13576457#comment-13576457 ] Pavel Yaskevich commented on CASSANDRA-4795: I don't think this is about right or wrong but rather about living by means instead of re-inventing broken bicycle. I'm not arguing or trying to convince, I'm simply saying that committed patch made situation even worse and it should be reconsidered all together. Also, if somebody ever had mistyped and didn't fix or used unrecognized attributes in replication strategy or compaction, after upgrade to 1.2.1 Cassandra just *wouldn't start up* which is also a ridiculously bad user experience, this is why I think this patch should be reverted. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13576690#comment-13576690 ] Jonathan Ellis commented on CASSANDRA-4795: --- bq. So in the case of Titan when they just want to save one 'last seen version' attribute, creating a separate column family or even having that per-cf doesn't make any sense but it's pretty convenient to keep it in the keypsace strategy options (without have any other alternatives) because it's not updated or even read that often. No doubt everyone practicing this extremely awful hack has a perfectly good excuse. :) But fundamentally creating a last_seen_versions table with cfname (ksname?) is simply not that onerous and is the Right Thing To Do. bq. patch 3 is my suggestion for an alternative to the problem I want to attach some tiny piece of metadata to a keyspace and creating a CF for that is overkill -1 from me, I get that change is painful for people who are abusing schema right now but we really shouldn't be encouraging this. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 0001-Reallow-unexpected-strategy-options-for-thrift.txt, 0002-Reallow-unexpected-strategy-options-for-thrift.txt, 0003-Adds-application_metadata-field-to-ks-metadata.txt, 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13577119#comment-13577119 ] Pavel Yaskevich commented on CASSANDRA-4795: bq. But fundamentally creating a last_seen_versions table with cfname (ksname?) is simply not that onerous and is the Right Thing To Do. For that specific case it's actually not worth it because it's always a single version as there is no reason to keep older version around, so creating separate CF for one row with empty value is overkill as specially as it's even not read that often and query wouldn't be as convenient as it was with describe keyspace for that single piece of data. bq. -1 from me, I get that change is painful for people who are abusing schema right now but we really shouldn't be encouraging this. It's usually one/two attributes that go there so I think it's pretty convenient to store it that way instead of dealing with separate CF/querying, but I'm welcome to new ideas replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 0001-Reallow-unexpected-strategy-options-for-thrift.txt, 0002-Reallow-unexpected-strategy-options-for-thrift.txt, 0003-Adds-application_metadata-field-to-ks-metadata.txt, 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13577121#comment-13577121 ] Jonathan Ellis commented on CASSANDRA-4795: --- Show me an existing database that lets you attach random crap to a table definition and I will reconsider my position. :) replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 0001-Reallow-unexpected-strategy-options-for-thrift.txt, 0002-Reallow-unexpected-strategy-options-for-thrift.txt, 0003-Adds-application_metadata-field-to-ks-metadata.txt, 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13575964#comment-13575964 ] Pavel Yaskevich commented on CASSANDRA-4795: Ok, so changing this in 1.2.1 probably was a bit unfair to people, because e.g. Titan relies on providing custom strategy option 'titan-version' and it can't do custom strategy because there is no control of what version people install, can we please start warning people about unrecognized options in the strategy again instead of restricting them, because those options are sometimes very convenient to safe meta-information. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13576438#comment-13576438 ] Sylvain Lebresne commented on CASSANDRA-4795: - bq. changing this in 1.2.1 probably was a bit unfair to people If you mean in a minor release, then I agree, mea culpa. But on the principle, I disagree that we should allow people to shove random metadata in the stategy options. Especially because that means not validating correctly the actual strategy options (and no, warning in the log is not a good enough because 1) application developer may not have easy access to the server log and 2) asking people to check the server log when they do query to see if they hadn't mispelled some option is a ridiculously bad user experience). If application want to store data, they should create a column family and store data in there. But to be clear, I'm open to discussing adding some new application_metadata field to the column family metadata that would explicitely be uninterpreted by Cassandra (not that I'm particularly thrilled by the idea, but I'm at least open to it), but the strategy options is just not the right place for that. So I'm fine reverting this from 1.2 (in which case I'd still prefer keeping the validation at least on the CQL3 side), not so much from trunk. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13576440#comment-13576440 ] Pavel Yaskevich commented on CASSANDRA-4795: bq. If you mean in a minor release, then I agree, mea culpa. That's exactly what I mean. bq. 1) application developer may not have easy access to the server log 2) asking people to check the server log when they do query to see if they hadn't mispelled some option is a ridiculously bad user experience If the original intention was to protect people from mistypes then maybe we need to put more work and throw exception only if typed in is similar to what it should be, like git does e.g. option rplicatian_factor is not recognized maybe you mean 'replication_factor'? ? Requiring those options to be typed in by the user especially with such a long names that we have already is a ridiculously bad user experience, especially when those names/options change frequently between releases. bq. If application want to store data, they should create a column family and store data in there. But to be clear, I'm open to discussing adding some new application_metadata field to the column family metadata that would explicitely be uninterpreted by Cassandra (not that I'm particularly thrilled by the idea, but I'm at least open to it), but the strategy options is just not the right place for that. So in the case of Titan when they just want to save one 'last seen version' attribute, creating a separate column family or even having that per-cf doesn't make any sense but it's pretty convenient to keep it in the keypsace strategy options (without have any other alternatives) because it's not updated or even read that often. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795_compaction_strategy_v3.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13560541#comment-13560541 ] Sylvain Lebresne commented on CASSANDRA-4795: - Patch looks good but a few remaining remarks/nits: * I think the comment in AbstractCompactionStrategy ctor is a bit confusing as we don't really fully repeat all the checks (we don't catch NumberFormatException nor validate the tombstoneThreshold value). So I agree we should probably repeat the checks, but let's repeat them fully. * In SizeTiered validation, we might want to catch NumberFormatException. * Since validateOptions is supposed to return a map of the options it don't know about, let's maybe check it's empty at the end of CFMetaData.validateCompactionOptions and get rid of the individual checks in Leveled and SizeTiered (which avoid needing VALID_OPTIONS). * Might I suggest adding a few static getInt(map, property_name, defaut_value), getDouble, ... helper methods to simplify stuff like {noformat} try { String optionValue = options.get(MIN_SSTABLE_SIZE_KEY); long minSSTableSize = optionValue == null ? DEFAULT_MIN_SSTABLE_SIZE : Long.parseLong(optionValue); } catch (NumberFormatException e) { ... } {noformat} * In cql3.CFPropDefs, we should move the validateCompactionOptions line inside the preceding 'if' statement. * In cql3.CreateColumnFamilyStatement, let's remove the validate method override since it's not doing anything. As a side note, there's a bit too much line changed not related to the patch to my taste. I understand this is your editor doing that automatically, but while reordering imports is ok, removing static imports to inline the class name in the code (in CFMetaData) is less justified imo. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13560671#comment-13560671 ] Dave Brosius commented on CASSANDRA-4795: - {quote}Since validateOptions is supposed to return a map of the options it don't know about, let's maybe check it's empty at the end of CFMetaData.validateCompactionOptions and get rid of the individual checks in Leveled and SizeTiered (which avoid needing VALID_OPTIONS).{quote} If user supplied compaction strategies derive from these classes, that won't work. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13560678#comment-13560678 ] Sylvain Lebresne commented on CASSANDRA-4795: - bq. If user supplied compaction strategies derive from these classes Why? tTe contract of validateOptions would be that it should return a list with *only* the options it doesn't know about (which is kind of what it is supposed to do (and does) in your patch already anyway). Thus if a user supplied compaction derives from an existing one, it would have to either 1) return an empty map if he wants to ignore on purpose options or 2) call the super class validateOptions first (which is what Leveled and SizeTiered do with AbstractStrategy). Of course, in the case where the compaction strategy do not implement a validateOptions, we should skip the check for invalid options, but that's no different from what the patch does now. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13560740#comment-13560740 ] Dave Brosius commented on CASSANDRA-4795: - yeah, your right. i shouldn't form opinions early in the morning. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795_compaction_strategy_v2.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13560359#comment-13560359 ] Dave Brosius commented on CASSANDRA-4795: - not sure where CFPropDefs.validate gets used with cql3 replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13553647#comment-13553647 ] Sylvain Lebresne commented on CASSANDRA-4795: - On the compaction strategy patch: * I would move the call to {{validateCompactionOptions}} in {{CFPropDefs.validate()}}. First because ALTER also need the validation and putting it in CFPropDefs handles both for free. And also because there is no guarantee that {{CFPropDefs.compactionStrategyClass}} won't be {{null}} which I think is not handled correctly by the patch (and moving things to {{CFPropDefs.validate()}} makes it easier to deal with). * We should not only validate that there is no unknown option provided, but we should also move the validation of the options themselves from the ctors to the validateOptions method. Also, I'd prefer adding a validateOptions to AbstractCompactionStrategy to handle the tombstone related option and have subclasses call that method explicitly rather than handling everything in the sublcasses. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.compaction_strategy.txt, 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13553475#comment-13553475 ] Dave Brosius commented on CASSANDRA-4795: - so, for compaction options, it appears to me that you can't create a CompactionStrategy object to do validation, because the ColumnFamilyStore needs to exist for the ACS constructor, which means it's too late to safe guard invalid options, right? The validation would need to be in CFMetaData i think, and CFMetaData would validate options needed by fetching the valid set from a static call thru reflection on the compactionStrategyClass which seems ugly... so am i missing something easier? replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13553488#comment-13553488 ] Jonathan Ellis commented on CASSANDRA-4795: --- What if we required a static {{validate}} method on ACS subclasses that we call with the options and maybe CFMetaData? Ugly, since there's no polymorphism on statics, but since we're doing ACS creation via reflection anyway it's not really a problem. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13553498#comment-13553498 ] Dave Brosius commented on CASSANDRA-4795: - the 'right'?? solution perhaps is to remove the cfs from AbstractCompactionStrategy ctor, and instead pass it into the various methods that need it. Problem there is that you'd need a 'postCreateInitialize(ColumnFamilyStore cfs)' on ACS, as SizeTiered and Leveled do things with cfs in the ctor, that would need to move to that method. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13553504#comment-13553504 ] Jonathan Ellis commented on CASSANDRA-4795: --- That's what I thought at first but ACS is free to maintain state as well, e.g. LCS's manifest. It would suck to turn those all into MapCFS, X. Alternativley you could split off the construction into a StrategyFactory, which IMO is clunkier than the reflection idea. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13553511#comment-13553511 ] Dave Brosius commented on CASSANDRA-4795: - ok, and just fail with ConfigurationException if that static method doesn't exist? replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13553517#comment-13553517 ] Jonathan Ellis commented on CASSANDRA-4795: --- I'd no-op it for backwards compatibility. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13550882#comment-13550882 ] Dave Brosius commented on CASSANDRA-4795: - yeah, sorry. fell asleep at the wheel. replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
[ https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13547832#comment-13547832 ] Sylvain Lebresne commented on CASSANDRA-4795: - +1 on part1, committed). Dave, are you still planning on working on the compaction and compression validation? replication, compaction, compression? options are not validated --- Key: CASSANDRA-4795 URL: https://issues.apache.org/jira/browse/CASSANDRA-4795 Project: Cassandra Issue Type: Bug Components: Core Affects Versions: 1.1.0 Reporter: Brandon Williams Assignee: Dave Brosius Priority: Minor Fix For: 1.2.1 Attachments: 4795.replication_strategy.txt When creating a keyspace and specifying strategy options, you can pass any k/v pair you like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira