[incubator-zipkin-brave] branch fix-sampler created (now a4dd38f)

[ad...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

adriancole pushed a change to branch fix-sampler
in repository https://gitbox.apache.org/repos/asf/incubator-zipkin-brave.git.


      at a4dd38f  Fixes some edge cases around sampler resetting

This branch includes the following new commits:

     new a4dd38f  Fixes some edge cases around sampler resetting

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

[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();