[
https://issues.apache.org/jira/browse/CASSANDRA-15718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17088124#comment-17088124
]
David Capwell commented on CASSANDRA-15718:
-------------------------------------------
bq. even though we know the distinctPartitions I'm not sure that we know
exactly what the returned value might be
{code}
public void update(long value)
{
long now = clock.getTime();
rescaleIfNeeded(now);
int index = findIndex(bucketOffsets, value);
updateBucket(decayingBuckets, index,
Math.round(forwardDecayWeight(now)));
updateBucket(buckets, index, 1);
}
public long getMax()
{
final int lastBucket = decayingBuckets.length - 1;
if (decayingBuckets[lastBucket] > 0)
return Long.MAX_VALUE;
for (int i = lastBucket - 1; i >= 0; i--)
{
if (decayingBuckets[i] > 0)
return bucketOffsets[i];
}
return 0;
}
{code}
Yep, we loose the original value and min/max actually reflect the bucket, so we
would need to know the bucketing to actually be able to correctly assert the
value. The only clean way I can see is if the snapshot had a method to convert
a value to bucket, that would at least let us make sure we saw data for the
correct bucket.
> 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: [email protected]
For additional commands, e-mail: [email protected]
