[incubator-zipkin-brave] 01/01: Fixes some edge cases around sampler resetting

[ad...@apache.org]

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 <ac...@pivotal.io>
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();