[ 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