KeDeng has posted comments on this change. ( http://gerrit.cloudera.org:8080/19918 )
Change subject: [java] add buffer space limit for KuduSession ...................................................................... Patch Set 6: (8 comments) Thanks for your reviews. http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java: http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@625 PS5, Line 625: !activeBuffer.isEmpty() | > nit: !activeBuffer.isEmpty() Done http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@751 PS5, Line 751: (activeBufferOps + 1 >= mutationBufferMaxOps || : isAboveMaxSize(activeBufferSize + operation.getRow().size() > How about encapsulate a function like 'boolean needFlush(int activeBufferOp I'm afraid doing so will make the code more complex. At present, the logic of L751 is more complex than the other two, and L751 and L729 are different scenarios in the same mode, which cannot be completely covered by the functions of the two parameters. If we insist on doing so, it may not be as simple as the logic discriminator. http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@893 PS5, Line 893: > nit: add some comments to clarify it's not the operations.size() Done http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@980 PS5, Line 980: public String toString() { > How about output the 'operationSize' as well? Done http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java: http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1912 PS5, Line 1912: @return in memory size of this row. > To make it work well with javadoc, it would be great to format the comments Done http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1914 PS5, Line 1914: : > nit: remove the space. Done http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java File java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java: http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java@98 PS5, Line 98: byte > nit: byte size Done http://gerrit.cloudera.org:8080/#/c/19918/5/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java@100 PS5, Line 100: byte > nit: byte size Done -- To view, visit http://gerrit.cloudera.org:8080/19918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I312d47c98566f9405361d969a4b68b326bb3c4d9 Gerrit-Change-Number: 19918 Gerrit-PatchSet: 6 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Tue, 13 Jun 2023 07:37:41 +0000 Gerrit-HasComments: Yes
