You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by aweisberg <gi...@git.apache.org> on 2018/09/06 14:11:57 UTC
[GitHub] cassandra pull request #258: DecayingEstimatedHistogramReservoir.EstimatedHi...
GitHub user aweisberg opened a pull request:
https://github.com/apache/cassandra/pull/258
DecayingEstimatedHistogramReservoir.EstimatedHistogramReservoirSnapshot returns wrong value for size() and incorrectly calculates count
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aweisberg/cassandra cassandra-14696-trunk
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cassandra/pull/258.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #258
----
commit 7913d60ad1b245dff4c972cfb0e2ee52b898232e
Author: Ariel Weisberg <aw...@...>
Date: 2018-09-05T17:59:20Z
Debug transient ring replace
commit 978e95f0629cc847e620f403fb628e4d6c4d7472
Author: Ariel Weisberg <aw...@...>
Date: 2018-09-05T21:35:47Z
DecayingEstimatedHistogramReservoir.EstimatedHistogramReservoirSnapshot returns wrong value for size() and incorrectly calculates count
Patch by Ariel Weisberg; Reviewed by Chris Lohfink for CASSANDRA-14696
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #258: DecayingEstimatedHistogramReservoir.EstimatedHi...
Posted by aweisberg <gi...@git.apache.org>.
Github user aweisberg commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/258#discussion_r215695991
--- Diff: src/java/org/apache/cassandra/db/ColumnFamilyStore.java ---
@@ -453,8 +454,12 @@ public void updateSpeculationThreshold()
{
try
{
- sampleReadLatencyNanos = metadata().params.speculativeRetry.calculateThreshold(metric.coordinatorReadLatency);
- transientWriteLatencyNanos = metadata().params.speculativeWriteThreshold.calculateThreshold(metric.coordinatorWriteLatency);
+ Snapshot readLatencySnapshot = metric.coordinatorReadLatency.getSnapshot();
+ if (readLatencySnapshot.size() > 0)
+ sampleReadLatencyNanos = metadata().params.speculativeRetry.calculateThreshold(readLatencySnapshot);
+ Snapshot writeLatencySnapshot = metric.coordinatorWriteLatency.getSnapshot();
+ if (writeLatencySnapshot.size() > 0)
+ transientWriteLatencyNanos = metadata().params.speculativeWriteThreshold.calculateThreshold(writeLatencySnapshot);
--- End diff --
You are right for fixed policies it does the wrong thing. Good catch.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #258: DecayingEstimatedHistogramReservoir.EstimatedHi...
Posted by aweisberg <gi...@git.apache.org>.
Github user aweisberg closed the pull request at:
https://github.com/apache/cassandra/pull/258
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra issue #258: DecayingEstimatedHistogramReservoir.EstimatedHistogram...
Posted by aweisberg <gi...@git.apache.org>.
Github user aweisberg commented on the issue:
https://github.com/apache/cassandra/pull/258
Committed as https://github.com/apache/cassandra/commit/8d443805f06e7abb25f768f6c800b7ae71bd4a41
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #258: DecayingEstimatedHistogramReservoir.EstimatedHi...
Posted by clohfink <gi...@git.apache.org>.
Github user clohfink commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/258#discussion_r215644267
--- Diff: src/java/org/apache/cassandra/db/ColumnFamilyStore.java ---
@@ -453,8 +454,12 @@ public void updateSpeculationThreshold()
{
try
{
- sampleReadLatencyNanos = metadata().params.speculativeRetry.calculateThreshold(metric.coordinatorReadLatency);
- transientWriteLatencyNanos = metadata().params.speculativeWriteThreshold.calculateThreshold(metric.coordinatorWriteLatency);
+ Snapshot readLatencySnapshot = metric.coordinatorReadLatency.getSnapshot();
+ if (readLatencySnapshot.size() > 0)
+ sampleReadLatencyNanos = metadata().params.speculativeRetry.calculateThreshold(readLatencySnapshot);
+ Snapshot writeLatencySnapshot = metric.coordinatorWriteLatency.getSnapshot();
+ if (writeLatencySnapshot.size() > 0)
+ transientWriteLatencyNanos = metadata().params.speculativeWriteThreshold.calculateThreshold(writeLatencySnapshot);
--- End diff --
Since the size > 0 is really a precondition for calculateThreshold to run successfully, it seems like the check should be there instead in the policies that require it vs an implicit undocumented precondition to be checked before in only the existing calls (incase a new entry point ever created).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org