Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17773 )
Change subject: KUDU-1260: Fix prefetching bug on Java scanner ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/17773/4/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/17773/4/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@290 PS4, Line 290: * Every time the client calls nextRows(), a new RPC together with a prefetching RPC : * are sent to the server side, so the queue will not have more than 2 items. Thank you for adding this. However, I can make sense of this explanation. From the code I can see that a deferred is added into the queue when a callback is invoked for an RPC, whether it's the very first RPC to fetch rows or one of the pre-fetching ones which are sent after the previous RPC returns with results. So, how we can guarantee that if the queue has gotten 2 elements already at some point, the prefetch callback cannot be called at least at for one of the elements, trying to add 3rd element into the queue? Could you clarify on that, please? http://gerrit.cloudera.org:8080/#/c/17773/4/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@642 PS4, Line 642: Deferred<RowResultIterator> prefetcherDeferred = cachedPrefetcherDeferred.poll(); : prefetcherDeferred.chain(new Deferred<RowResultIterator>().addCallback(prefetch)); In the case when 'cachedPrefetcherDeferred' contains 2 elements (if ever), should the next prefetch callback be chained not to the head, but to the tail element of the queue instead? -- To view, visit http://gerrit.cloudera.org:8080/17773 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I853a041d86c75ec196d7d4ff45af4673c5c5f5cd Gerrit-Change-Number: 17773 Gerrit-PatchSet: 4 Gerrit-Owner: Hongjiang Zhang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hongjiang Zhang <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 19 Aug 2021 02:43:15 +0000 Gerrit-HasComments: Yes
