Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Fix GetTableLocations error handling ......................................................................
Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/3295/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java: Line 764: * "Errback" used to delayed-retry a RPC if it fails due to no leader master being found. Javadoc needs to be modified. Line 793: request.errback(arg); I was surprised originally to see this issue, but it makes sense. The errback in this case is added on the GetTableLocationsResponse, not the actual RPC. I wonder if we have other cases like this where we assume we are sending the errback all the way to the client., but in reality they get lost. http://gerrit.cloudera.org:8080/#/c/3295/2/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java File java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java: PS2, Line 113: RPC get stuck. nit: to* get stuck. Line 119: session.setFlushMode(AsyncKuduSession.FlushMode.AUTO_FLUSH_SYNC); Isn't that line unnecessary? Line 126: Thread.sleep(2000); Can we find another way to find when the tablet is deleted to not sleep for 2 seconds? PS2, Line 131: cause table is deleted. nit: because the table was deleted. Line 132: e.printStackTrace(); Why print it if it's expected? -- To view, visit http://gerrit.cloudera.org:8080/3295 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79d246a83e9f0071063a23fb9a8d4b4c0220e507 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Binglin Chang <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
