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

Ekaterina Dimitrova edited comment on CASSANDRA-17677 at 6/2/22 8:31 PM:
-------------------------------------------------------------------------

I feel we lack testing of config for the BulkLoader.

The LoaderOptions test is there indeed, but while BulkLoaderTest exists, it 
doesn't exhaust all the config and this seems risky to me.

I just also noticed CASSANDRA-17062 which is  another config load problem in 
the same area.

There was huge effort to test required config parameters of tools in 4.0.

I think it is time to add additional testing for other config too. 

[~dcapwell], what do you think? I know you were actually reviewing those 
testing efforts so you probably have a point of view. Also, we need second 
committer reviewer, if you have time. :)  

 

About the currently published patch - I did a quick skim and things look good 
to me so far. The only thing I miss is BulkLoader test that the config is 
loaded properly as LoaderOptionsTest checks only the parsing as we already 
noticed. We also need full CI run. 


was (Author: e.dimitrova):
I feel we lack testing of config for the BulkLoader.

The LoaderOptions test is there indeed, but while BulkLoaderTest exists, it 
doesn't exhaust all the config and this seems risky to me.

I just also noticed CASSANDRA-17062 which is  another config load problem in 
the same area.

There was huge effort to test required config parameters of tools in 4.0.

I think it is time to add additional testing for other config too. 

[~dcapwell], what do you think? I know you were actually reviewing those 
testing efforts so you probably have a point of view. Also, we need second 
committer reviewer, if you have time. :)  

 

About the currently published patch - I did a quick skim and things look good 
to me so far. The only thing I miss is BulkLoader test that the config is 
loaded properly as LoaderOptionsTest checks only the parsing as we already 
noticed. We also need full CI. 

> Fix BulkLoader to load  entireSSTableThrottle and entireSSTableInterDcThrottle
> ------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-17677
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-17677
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Tool/bulk load
>            Reporter: Ekaterina Dimitrova
>            Assignee: Francisco Guerrero
>            Priority: Normal
>             Fix For: 4.1-beta, 4.1.x, 4.x
>
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> {{entire_sstable_stream_throughput_outbound and 
> entire_sstable_inter_dc_stream_throughput_outbound}} were introduced in 
> CASSANDRA-17065.They were added to the LoaderOptions class but they are not 
> loaded in BulkLoader as {{throttle}} and {{interDcThrottle are. }}{{As part 
> of this ticket we need to fix the BulkLoader, also those properties should be 
> advertised as MiB/s, not megabits/s. This was not changed in CASSANDRA-15234 
> for the bulk loader because those are not loaded and those variables in 
> LoaderOptions are disconnected from the Cassandra config parameters and 
> unused at the moment. }}
> It will be good also to update the doc here - 
> [https://cassandra.apache.org/doc/latest/cassandra/operating/bulk_loading.html,|https://cassandra.apache.org/doc/latest/cassandra/operating/bulk_loading.html]
> {{and add a test that those are loaded properly when used with the 
> BulkLoader. }}
> {{CC [~frankgh] }}



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to