[
https://issues.apache.org/jira/browse/CASSANDRA-4795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
