Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12715 )
Change subject: [java] Make the KuduScanner iterable ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/12715/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12715/2//COMMIT_MSG@18 PS2, Line 18: changed > Nit: changes Done http://gerrit.cloudera.org:8080/#/c/12715/2//COMMIT_MSG@19 PS2, Line 19: craete > create Done http://gerrit.cloudera.org:8080/#/c/12715/2//COMMIT_MSG@20 PS2, Line 20: while still sharing the underlying : data. > IIUC, you get one RowResultIterator for every Scan RPC response, and one Ro The row data returned from a Scan RPC is passed by reference to the RowResult and then the RowResult has an index to its row in that data. Every-time a new Scan RPC Response, and therefore RowResultIterator, is created a new Slice/array is used for the new underlying data. Java won't GC any of the batch data until no references are still being used. Once the last RowResult for a batch is GCd, so will the underlying data. I will have tests that prove this. http://gerrit.cloudera.org:8080/#/c/12715/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java: http://gerrit.cloudera.org:8080/#/c/12715/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@1030 PS2, Line 1030: // TODO: Find a clean way to plumb in reuseRowResult. > ScanRequest is a nested class, so it has a reference to the AsyncKuduScanne The challenge here is that I would need to add this property to the scanToken to make it work with the scanToken API. Since this is a Java only thing that didn't really make sense to me. I think we have bloated the scanToken proto already. In a perfect world I think a scanToken would define *what* to scan, and some other configs could define *how*. I could provide a setter method on the Scanner object after it's created, it's just not my favorite style. http://gerrit.cloudera.org:8080/#/c/12715/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java: http://gerrit.cloudera.org:8080/#/c/12715/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java@31 PS2, Line 31: public class KuduScanner implements Iterable<RowResult> { > Should AsyncKuduScanner also be Iterable? I could, but a use wasn't really clear. http://gerrit.cloudera.org:8080/#/c/12715/2/java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java File java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java: http://gerrit.cloudera.org:8080/#/c/12715/2/java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java@65 PS2, Line 65: reUseRowResult > Nit: reuseRowResult would be a tad more clear. Done -- To view, visit http://gerrit.cloudera.org:8080/12715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e4ac59e30d0562c0a381d5e304af1dcfdcf5a1a Gerrit-Change-Number: 12715 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Tue, 12 Mar 2019 14:11:02 +0000 Gerrit-HasComments: Yes
