[
https://issues.apache.org/jira/browse/CASSANDRA-15718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17088140#comment-17088140
]
David Capwell commented on CASSANDRA-15718:
-------------------------------------------
*
https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15582-trunk-batchmetrics#diff-8948cec1f9d33f10b15c38de80141548R170.
Should call snapshot once, else you do more memory copies than needed.
*
https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15582-trunk-batchmetrics#diff-8948cec1f9d33f10b15c38de80141548R133
Can we remove ".withExamples(10)", should rely on the default. We currently
disable shrinking so that won't case the GC to freak out. If you also fix the
above statement you drop the amount of memory considerably (clone 9 times per
iteration, could be 1 time).
*
https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15582-trunk-batchmetrics#diff-8948cec1f9d33f10b15c38de80141548R178
this doesn't look stable it assumes the very first test case has 1 partition,
if the first test case has 2 or more partitions then it should fail (since the
value is now 2 [1]).
* Would be good to call
"org.apache.cassandra.metrics.DecayingEstimatedHistogramReservoir#clear" so
each test doesn't see the results of the previous tests.
For min/max the only thing I can think of is to expose a test method to get the
bucket for a specific value, that would let us refine min/max. Something like
the below would work
{code}
class EstimatedHistogramReservoirSnapshot
...
public long getBucketValue(long value)
{
int index = findIndex(bucketOffsets, value);
return bucketOffsets[index];
}
{code}
[~jrwest] since you have worked with this recently; thoughts? Should we leave
the max as is?
[1] - here are the first 30 buckets:
{code}
0 = 1
1 = 2
2 = 3
3 = 4
4 = 5
5 = 6
6 = 7
7 = 8
8 = 10
9 = 12
10 = 14
11 = 17
12 = 20
13 = 24
14 = 29
15 = 35
16 = 42
17 = 50
18 = 60
19 = 72
20 = 86
21 = 103
22 = 124
23 = 149
24 = 179
25 = 215
26 = 258
27 = 310
28 = 372
29 = 446
{code}
> Improve BatchMetricsTest
> -------------------------
>
> Key: CASSANDRA-15718
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15718
> Project: Cassandra
> Issue Type: Improvement
> Components: Test/unit
> Reporter: Stephen Mallette
> Assignee: Stephen Mallette
> Priority: Normal
>
> As noted in CASSANDRA-15582 {{BatchMetricsTest}} should test
> {{BatchStatement.Type.COUNTER}} to cover all the {{BatchMetrics}}. Some
> changes were introduced to make this improvement at:
> https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15582-trunk-batchmetrics
> and the following suggestions were made in review (in addition to the
> suggestion that a separate JIRA be created for this change) by [~dcapwell]:
> {quote}
> * 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
> {quote}
--
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
