This is an automated email from the ASF dual-hosted git repository. adriancole pushed a commit to branch fix-sampler in repository https://gitbox.apache.org/repos/asf/incubator-zipkin-brave.git
commit a4dd38f9d575522867d22381f03a533927124d30 Author: Adrian Cole <[email protected]> AuthorDate: Sat May 11 22:36:36 2019 +0800 Fixes some edge cases around sampler resetting I found these working on https://github.com/adriancole/zipkin-voltdb/pull/18 I don't have unit tests for the changes, but integrated looks a bit right now. I think @pburm might be interested. --- .../src/main/java/brave/sampler/RateLimitingSampler.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/brave/src/main/java/brave/sampler/RateLimitingSampler.java b/brave/src/main/java/brave/sampler/RateLimitingSampler.java index 7bb775e..a6a3ef4 100644 --- a/brave/src/main/java/brave/sampler/RateLimitingSampler.java +++ b/brave/src/main/java/brave/sampler/RateLimitingSampler.java @@ -55,14 +55,17 @@ public class RateLimitingSampler extends Sampler { long updateAt = nextReset.get(); long nanosUntilReset = -(now - updateAt); // because nanoTime can be negative - boolean shouldReset = nanosUntilReset <= 0; - if (shouldReset) { - if (nextReset.compareAndSet(updateAt, updateAt + NANOS_PER_SECOND)) { - usage.set(0); - } + if (nanosUntilReset <= 0) { + // Attempt to move into the next sampling interval. + // nanosUntilReset is now invalid regardless of race winner, so we can't sample based on it. + if (nextReset.compareAndSet(updateAt, updateAt + NANOS_PER_SECOND)) usage.set(0); + + // recurse as it is simpler than resetting all the locals. + // reset happens once per second, this code doesn't take a second, so no infinite recursion. + return isSampled(ignoredTraceId); } - int max = maxFunction.max(shouldReset ? 0 : nanosUntilReset); + int max = maxFunction.max(nanosUntilReset); int prev, next; do { // same form as java 8 AtomicLong.getAndUpdate prev = usage.get();
