Github user kl0u commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4993#discussion_r150856825
  
    --- Diff: 
flink-queryable-state/flink-queryable-state-client-java/src/main/java/org/apache/flink/queryablestate/network/Client.java
 ---
    @@ -483,27 +511,31 @@ private boolean close(Throwable cause) {
                @Override
                public void onRequestResult(long requestId, RESP response) {
                        TimestampedCompletableFuture pending = 
pendingRequests.remove(requestId);
    -                   if (pending != null && pending.complete(response)) {
    +                   if (pending != null && !pending.isDone()) {
                                long durationMillis = (System.nanoTime() - 
pending.getTimestamp()) / 1_000_000L;
                                stats.reportSuccessfulRequest(durationMillis);
    --- End diff --
    
    This was creating test instabilities in the 
`AbstractServerTest.testPortRangeSuccess()`. This is why I decided to complete 
the future after increasing the counter. Given that we check if the 
`pending.isDone()`, we do not leave much room for false increases. 


---

Reply via email to