Adar Dembo has posted comments on this change.

Change subject: [java client] Fix a race in TabletClient cleanup
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3340/2/java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
File java/kudu-client/src/main/java/org/kududb/client/TabletClient.java:

Line 203:     if (tryAgain) {
Given the flow of L177-197, there's no way that both failRpc and tryAgain can 
be true, so the old code of "if failRpc else if tryAgain" is fine. Is there 
another reason for changing this down here that I'm not seeing?


Line 678:       rpcs = pending_rpcs == null ? new 
ArrayList<KuduRpc<?>>(rpcs_inflight.size()) : pending_rpcs;
Nit: can you use "new ArrayList<>(...)" here?


Line 685:     if (rpcs != null) {
Doesn't seem like rpcs could ever be non-null. Maybe you mean to check if it's 
empty here?


-- 
To view, visit http://gerrit.cloudera.org:8080/3340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic87425bd54e01a50c3fc11f2862a3e5f737f2bf7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

Reply via email to