Adar Dembo has posted comments on this change.

Change subject: [java client] RPCs can get lost in a TabletClient race

Patch Set 1:


What releases (if any) are affected by this?

We've been applying bandaid after bandaid for these concurrency issues, and the 
lack of regression tests is increasingly concerning. Is there something missing 
from the client that'd make writing such tests easier?
File java/kudu-client/src/main/java/org/kududb/client/

PS1, Line 187: We got disconnected, cleanup ran, then we added the rpc to 
rpcs_inflight, so we need to
             :           // manually failOrRetry it.
I think this is too vague. Multiple threads are at work here, right? It's not 
clear what the race is from this terse description. Can you rewrite with a 
concrete example?

PS1, Line 678: // Removing from the iterator because ConcurrentHashMap doesn't 
have a way to retrieve and
             :       // remove all the entries atomically.
             :       for (Iterator<KuduRpc<?>> iterator = 
rpcs_inflight.values().iterator(); iterator.hasNext();) {
             :         KuduRpc<?> rpc =;
             :         rpcs.add(rpc);
             :         iterator.remove();
             :       }
I don't get it; how is this any more "atomic" than the previous code? What am I 

Line 685:       // After this, rpcs_inflight might still have entries since it 
could have been added
Nit: it -> they

Line 686:       // concurrently, and those RPCs will be handled by their called 
in sendRpc.
Nit: "their called"?

Maybe just rewrite this paragraph to make it more clear.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaff89eb832d0d6f0dede198661856fae1a8585a0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

Reply via email to