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. > If the usage is like: Right -- every Deferred in the queue has its prefetch callback, and the callback adds an entry into the queue. So, my question is: once there are two entries in the queue already, is it possible the the callback for the added Deferred called before nextRows() called by the user's code, added another entry in the queue, and so on? I don't see there is anything that could prevent that. And if so, then this updated prefetching code might fire exceptions once the queue cannot accommodate an extra element. 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)); > Here I want to guarantee every dequeued Deferred chains a prefetch. Right, but the next prefetching makes sense only after the latest prefetch has completed, right? Otherwise (as is with current code in PS4), there might be two prefetching in progress, which might not be a good idea since each induces extra IO on the server, but the client will receive the data in sequential order anyways due to the nextRows() API. -- 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 19:08:14 +0000 Gerrit-HasComments: Yes
