[jira] [Comment Edited] (CASSANDRA-18486) LeveledCompactionStrategy does not check its constructor

["Manish Ghildiyal (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-18486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17720333#comment-17720333 ] 

Manish Ghildiyal edited comment on CASSANDRA-18486 at 5/7/23 3:54 PM:
----------------------------------------------------------------------

I have created PR:

[https://github.com/apache/cassandra/pull/2311]

Please have a look.


was (Author: manish.c.ghildiyal@gmail.com):
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<String, String> validateOptions(Map<String, String> options) throws ConfigurationException
>     {
>         Map<String, String> 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<String, String> 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-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org