Adar Dembo has posted comments on this change. Change subject: [java-client] refactor AsyncKuduSession ......................................................................
Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java: Line 563: doFlush(fullBuffer); > Not sure, but I don't think doFlush can throw. In general, methods that re I think doFlush() may throw (unchecked excpetions) if the buffer passed to it was empty, in which case it'd callback() on the global flushNotification deferred, which would potentially run user code that would throw. It doesn't seem like that could happen from here (where fullBuffer is, well, full) but I wonder what that means for other doFlush() callers. http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/Batch.java File java/kudu-client/src/main/java/org/kududb/client/Batch.java: Line 46: final List<Operation> operations = new ArrayList<>(125); > nothing, and in fact it should probably be more like 125 (default buffer si So why bother predicting it at all? Anyway, if you do retain the preallocation, please add a comment explaining why 125 was chosen. -- To view, visit http://gerrit.cloudera.org:8080/3477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <d...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <d...@cloudera.com> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
