cassandra-commits mailing list archives

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


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
    String optionValue = options.get(MIN_SSTABLE_SIZE_KEY);
    long minSSTableSize = optionValue == null ? DEFAULT_MIN_SSTABLE_SIZE : Long.parseLong(optionValue);
catch (NumberFormatException e)
* 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

> replication, compaction, compression? options are not validated
> ---------------------------------------------------------------
>                 Key: CASSANDRA-4795
>                 URL:
>             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

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:

View raw message