Adar Dembo has posted comments on this change. Change subject: [java client] Few ITClient improvements ......................................................................
Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/4489/1/java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java File java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java: Line 155: scanner.close(); Hmm, didn't know we need to explicitly close. Guess I'm too used to the C++ client. PS1, Line 173: KuduScanner scanner = scanBuilder.build(); : while (scanner.hasMoreRows()) { : count += scanner.nextRows().getNumRows(); : } : return count; Can you use countRowsInScan() here? Maybe elsewhere too? http://gerrit.cloudera.org:8080/#/c/4489/1/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java: PS1, Line 107: thread.setUncaughtExceptionHandler(uncaughtExceptionHandler); You find this cleaner than explicitly catching RuntimeException in writerThread and scannerThread (is this an issue for chaosThread too)? PS1, Line 290: if (resp == null) { : return false; : } Why would the result of session.apply() be null, or session.flush() include a null in the list? Line 295: if (writeTimestamp != 0) { Under what conditions will it be 0? The very first write? PS1, Line 419: so when we get back with the same one Nit: this fragment is confusing. What does it mean to "get back with the same one"? -- To view, visit http://gerrit.cloudera.org:8080/4489 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b5ddca26b66e9fc1f737aaacf98df340f0b9024 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
