Passes the build

Author: j-baker

okhttp-clients/src/main/java/com/palantir/remoting3/okhttp/RemotingConcurrencyLimiter.java

@@ -43,7 +43,8 @@
  * <ol>
  * <li>Fix concurrency bug - upstream has a race between starting finishing the window and running requests.</li>
  * <li>Store the last update time as well as the next update time.</li>
- * <li>Use the last update time to pre-emptively close the window when a request is dropped, instead of waiting.</li>
+ * <li>Use the last update time to pre-emptively close the window when a request is dropped, instead of waiting.
+ *     Do this before handing back any tokens.</li>
  * <li>Remove the builder, and instead have a static factory method.</li>
  * <li>Change code style to match Palantir baseline.</li>
  * </ol>
@@ -117,10 +118,10 @@ public void onIgnore() {
 
             @Override
             public void onDropped() {
-                inFlight.decrementAndGet();
-                token.release();
                 sample.getAndUpdate(current -> current.addDroppedSample(currentMaxInFlight));
                 maybeUpdateWindow(System.nanoTime(), () -> startTime > lastUpdateTime, unused -> true);
+                inFlight.decrementAndGet();
+                token.release();
             }
 
         });
@@ -156,7 +157,7 @@ private void updateWindow(long endTime, Predicate<ImmutableSampleWindow> shouldF
         }
     }
 
-    private boolean isWindowReady(ImmutableSampleWindow sample) {
-        return sample.getCandidateRttNanos() < Long.MAX_VALUE && sample.getSampleCount() > windowSize;
+    private boolean isWindowReady(ImmutableSampleWindow window) {
+        return window.getCandidateRttNanos() < Long.MAX_VALUE && window.getSampleCount() > windowSize;
     }
 }

okhttp-clients/src/test/java/com/palantir/remoting3/okhttp/ConcurrencyLimitersTest.java

@@ -56,4 +56,4 @@ public void testFifo() throws ExecutionException, InterruptedException {
         first.get().onIgnore();
         assertThat(second.isDone()).isTrue();
     }
-}
+}

okhttp-clients/src/test/java/com/palantir/remoting3/okhttp/FlowControlTest.java

@@ -51,12 +51,12 @@
  * to try different strategies.
  * <p>
  * It is run in CI, but only to prevent code breakages - this is in general an expensive test which should be run
- * as a dev tool.
+ * as a dev tool. If you want to run for dev purposes, please increase REQUESTS_PER_THREAD.
  */
 public final class FlowControlTest {
     private static final Logger log = LoggerFactory.getLogger(FlowControlTest.class);
     private static final Duration GRACE = Duration.ofMinutes(2);
-    private static final int REQUESTS_PER_THREAD = System.getenv("CI") == null ? 1000 : 1;
+    private static final int REQUESTS_PER_THREAD = 5;
     private static final ConcurrencyLimiters limiters = new ConcurrencyLimiters();
     private static ListeningExecutorService executorService;
 
@@ -107,7 +107,7 @@ private Stream<Worker> createWorkers(
                         avgRetries));
     }
 
-    private static class Worker implements Runnable {
+    private static final class Worker implements Runnable {
         private final Supplier<BackoffStrategy> backoffFactory;
         private final ConcurrencyLimiter limiter;
         private final Duration successDuration;