Jean-Daniel Cryans has posted comments on this change.

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

Patch Set 1:

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 n

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 
It's not more atomic, we get around this by assuming we might be missing some 
rpcs that got added while iterating, hence the check in sendRpc above.

I removed the comment, it's not really making things clearer.

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"?

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: Jean-Daniel Cryans <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

Reply via email to