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

David Capwell commented on CASSANDRA-15582:
-------------------------------------------

For 
https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15582-trunk-batchmetrics

* I like the usage of BatchStatement.Type for the tests
* honestly feel quick theories is better than random, but glad you added the 
seed to all asserts =).  Would still be better as a quick theories test since 
you basically wrote a property anyways!
* 
https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15582-trunk-batchmetrics#diff-8948cec1f9d33f10b15c38de80141548R131
 feel you should rename to 
expectedPartitionsPerLoggedBatch{Count,Logged,Unlogged}.  pre is what the value 
is, post is what the value is expected to be (rather than what it is).
* 
https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15582-trunk-batchmetrics#diff-8948cec1f9d33f10b15c38de80141548R150
 this doesn't look correct.  the batch has distinctPartitions mutations, so 
shouldn't max reflect that?  I ran the current test in a debugger and see that 
that is the case (aka current test is wrong).

most of the comments are nit picks, but the last one looks like a test bug to me

> 4.0 quality testing: metrics
> ----------------------------
>
>                 Key: CASSANDRA-15582
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-15582
>             Project: Cassandra
>          Issue Type: Task
>          Components: Test/dtest
>            Reporter: Josh McKenzie
>            Assignee: Romain Hardouin
>            Priority: Normal
>             Fix For: 4.0-rc
>
>         Attachments: Screen Shot 2020-04-07 at 5.47.17 PM.png
>
>
> In past releases we've unknowingly broken metrics integrations and introduced 
> performance regressions in metrics collection and reporting. We strive in 4.0 
> to not do that. Metrics should work well!



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

Reply via email to