cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-4795) replication, compaction, compression? options are not validated
Date Wed, 23 Jan 2013 10:24:13 GMT

    [ 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

Mime
View raw message