[jira] [Commented] (CASSANDRA-18486) LeveledCompactionStrategy does not check its constructor
[
https://issues.apache.org/jira/browse/CASSANDRA-18486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17720473#comment-17720473
]
Manish Ghildiyal commented on CASSANDRA-18486:
--
[~maxwellguo] , thanks for you inputs. Seems changes are not even needed.
I would close PR.
> LeveledCompactionStrategy does not check its constructor
>
>
> Key: CASSANDRA-18486
> URL: https://issues.apache.org/jira/browse/CASSANDRA-18486
> Project: Cassandra
> Issue Type: Bug
> Components: Local/Compaction/LCS
>Reporter: Hao Zhong
>Assignee: Manish Ghildiyal
>Priority: Normal
> Fix For: 5.x
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> LeveledCompactionStrategy has the following code:
>
> {code:java}
> public static Map validateOptions(Map
> options) throws ConfigurationException
> {
> Map uncheckedOptions =
> AbstractCompactionStrategy.validateOptions(options);
> String size = options.containsKey(SSTABLE_SIZE_OPTION) ?
> options.get(SSTABLE_SIZE_OPTION) : "1";
> try
> {
> int ssSize = Integer.parseInt(size);
> if (ssSize < 1)
> {
> throw new ConfigurationException(String.format("%s must be
> larger than 0, but was %s", SSTABLE_SIZE_OPTION, ssSize));
> }
> }
> catch (NumberFormatException ex)
> {
> throw new ConfigurationException(String.format("%s is not a
> parsable int (base10) for %s", size, SSTABLE_SIZE_OPTION), ex);
> }
> ...
> } {code}
> This method throws ConfigurationException when the configuration file is
> wrong. The thrown exception is easy to understand, and calling code can catch
> this exception to handle configuration files with wrong formats. However, the
> constructor of this class does not check configuration files:
>
>
>
> {code:java}
> public LeveledCompactionStrategy(ColumnFamilyStore cfs, Map
> options)
> {...
>
> configuredMaxSSTableSize = Integer.parseInt(options.get(SSTABLE_SIZE_OPTION));
> ...
> configuredLevelFanoutSize =
> Integer.parseInt(options.get(LEVEL_FANOUT_SIZE_OPTION));
> ...
> }
> {code}
> As a result, this class can throw NumberFormatException when configuration
> files are wrong, and will not throw ConfigurationException.
> It is strange for calling code to call the static validation method before
> calling its constructor. Can this problem be fixed?
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-18486) LeveledCompactionStrategy does not check its constructor
[
https://issues.apache.org/jira/browse/CASSANDRA-18486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17720413#comment-17720413
]
maxwellguo commented on CASSANDRA-18486:
my question is , why we need to modify this ? as the way to modify compaction
strategy is through ddl when create or alter table.
but as [~maedhroz]have described , c* will do validate options.
Besides [~manish.c.ghildi...@gmail.com] I think your pr is not very suitable，
at the very least, if we need to modify, we can't just modify LCS, STWCS AND
TWCS may also need.
> LeveledCompactionStrategy does not check its constructor
>
>
> Key: CASSANDRA-18486
> URL: https://issues.apache.org/jira/browse/CASSANDRA-18486
> Project: Cassandra
> Issue Type: Bug
> Components: Local/Compaction/LCS
>Reporter: Hao Zhong
>Assignee: Manish Ghildiyal
>Priority: Normal
> Fix For: 5.x
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> LeveledCompactionStrategy has the following code:
>
> {code:java}
> public static Map validateOptions(Map
> options) throws ConfigurationException
> {
> Map uncheckedOptions =
> AbstractCompactionStrategy.validateOptions(options);
> String size = options.containsKey(SSTABLE_SIZE_OPTION) ?
> options.get(SSTABLE_SIZE_OPTION) : "1";
> try
> {
> int ssSize = Integer.parseInt(size);
> if (ssSize < 1)
> {
> throw new ConfigurationException(String.format("%s must be
> larger than 0, but was %s", SSTABLE_SIZE_OPTION, ssSize));
> }
> }
> catch (NumberFormatException ex)
> {
> throw new ConfigurationException(String.format("%s is not a
> parsable int (base10) for %s", size, SSTABLE_SIZE_OPTION), ex);
> }
> ...
> } {code}
> This method throws ConfigurationException when the configuration file is
> wrong. The thrown exception is easy to understand, and calling code can catch
> this exception to handle configuration files with wrong formats. However, the
> constructor of this class does not check configuration files:
>
>
>
> {code:java}
> public LeveledCompactionStrategy(ColumnFamilyStore cfs, Map
> options)
> {...
>
> configuredMaxSSTableSize = Integer.parseInt(options.get(SSTABLE_SIZE_OPTION));
> ...
> configuredLevelFanoutSize =
> Integer.parseInt(options.get(LEVEL_FANOUT_SIZE_OPTION));
> ...
> }
> {code}
> As a result, this class can throw NumberFormatException when configuration
> files are wrong, and will not throw ConfigurationException.
> It is strange for calling code to call the static validation method before
> calling its constructor. Can this problem be fixed?
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-18486) LeveledCompactionStrategy does not check its constructor
[
https://issues.apache.org/jira/browse/CASSANDRA-18486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17720333#comment-17720333
]
Manish Ghildiyal commented on CASSANDRA-18486:
--
I have created [PR|[https://github.com/apache/cassandra/pull/2311].]
Please have a look.
> LeveledCompactionStrategy does not check its constructor
>
>
> Key: CASSANDRA-18486
> URL: https://issues.apache.org/jira/browse/CASSANDRA-18486
> Project: Cassandra
> Issue Type: Bug
> Components: Local/Compaction/LCS
>Reporter: Hao Zhong
>Assignee: Manish Ghildiyal
>Priority: Normal
> Fix For: 5.x
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> LeveledCompactionStrategy has the following code:
>
> {code:java}
> public static Map validateOptions(Map
> options) throws ConfigurationException
> {
> Map uncheckedOptions =
> AbstractCompactionStrategy.validateOptions(options);
> String size = options.containsKey(SSTABLE_SIZE_OPTION) ?
> options.get(SSTABLE_SIZE_OPTION) : "1";
> try
> {
> int ssSize = Integer.parseInt(size);
> if (ssSize < 1)
> {
> throw new ConfigurationException(String.format("%s must be
> larger than 0, but was %s", SSTABLE_SIZE_OPTION, ssSize));
> }
> }
> catch (NumberFormatException ex)
> {
> throw new ConfigurationException(String.format("%s is not a
> parsable int (base10) for %s", size, SSTABLE_SIZE_OPTION), ex);
> }
> ...
> } {code}
> This method throws ConfigurationException when the configuration file is
> wrong. The thrown exception is easy to understand, and calling code can catch
> this exception to handle configuration files with wrong formats. However, the
> constructor of this class does not check configuration files:
>
>
>
> {code:java}
> public LeveledCompactionStrategy(ColumnFamilyStore cfs, Map
> options)
> {...
>
> configuredMaxSSTableSize = Integer.parseInt(options.get(SSTABLE_SIZE_OPTION));
> ...
> configuredLevelFanoutSize =
> Integer.parseInt(options.get(LEVEL_FANOUT_SIZE_OPTION));
> ...
> }
> {code}
> As a result, this class can throw NumberFormatException when configuration
> files are wrong, and will not throw ConfigurationException.
> It is strange for calling code to call the static validation method before
> calling its constructor. Can this problem be fixed?
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-18486) LeveledCompactionStrategy does not check its constructor
[
https://issues.apache.org/jira/browse/CASSANDRA-18486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17717751#comment-17717751
]
Caleb Rackliffe commented on CASSANDRA-18486:
-
{{validateOptions()}} is called in {{CompactionParams#validate()}} via
reflection, and I think the reason for this is that compaction strategies are
defined in DDL/on a table-by-table basis.
Do you have a specific proposal in mind?
> LeveledCompactionStrategy does not check its constructor
>
>
> Key: CASSANDRA-18486
> URL: https://issues.apache.org/jira/browse/CASSANDRA-18486
> Project: Cassandra
> Issue Type: Bug
> Components: Local/Compaction/LCS
>Reporter: Hao Zhong
>Priority: Normal
> Fix For: 5.x
>
>
> LeveledCompactionStrategy has the following code:
>
> {code:java}
> public static Map validateOptions(Map
> options) throws ConfigurationException
> {
> Map uncheckedOptions =
> AbstractCompactionStrategy.validateOptions(options);
> String size = options.containsKey(SSTABLE_SIZE_OPTION) ?
> options.get(SSTABLE_SIZE_OPTION) : "1";
> try
> {
> int ssSize = Integer.parseInt(size);
> if (ssSize < 1)
> {
> throw new ConfigurationException(String.format("%s must be
> larger than 0, but was %s", SSTABLE_SIZE_OPTION, ssSize));
> }
> }
> catch (NumberFormatException ex)
> {
> throw new ConfigurationException(String.format("%s is not a
> parsable int (base10) for %s", size, SSTABLE_SIZE_OPTION), ex);
> }
> ...
> } {code}
> This method throws ConfigurationException when the configuration file is
> wrong. The thrown exception is easy to understand, and calling code can catch
> this exception to handle configuration files with wrong formats. However, the
> constructor of this class does not check configuration files:
>
>
>
> {code:java}
> public LeveledCompactionStrategy(ColumnFamilyStore cfs, Map
> options)
> {...
>
> configuredMaxSSTableSize = Integer.parseInt(options.get(SSTABLE_SIZE_OPTION));
> ...
> configuredLevelFanoutSize =
> Integer.parseInt(options.get(LEVEL_FANOUT_SIZE_OPTION));
> ...
> }
> {code}
> As a result, this class can throw NumberFormatException when configuration
> files are wrong, and will not throw ConfigurationException.
> It is strange for calling code to call the static validation method before
> calling its constructor. Can this problem be fixed?
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
