Hongjiang Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17773 )

Change subject: KUDU-1260: Fix prefetching bug on Java scanner
......................................................................


Patch Set 6:

(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.
If the usage is like:
while(it.hasMore()) {
  rows = it.nextRows();
}

nextRows() always adds a prefetch callback after the gotNextRow callback, and 
dequeue a Deferred. After this Deferred's join() is called and that means RPC 
finished, its prefetch callback is triggered to create a new Deferred and 
append it to the queue.

It is possible that the RPC finished very fast, but the nextRows() has not yet 
been invoked. In this case, the queue has two items. This is also the root 
cause of the original bug.

As you can see, only the dequeued Deferred chains the prefetch callback. It 
avoids two consecutive prefetching callbackes were chained.


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),
Here I want to guarantee every dequeued Deferred chains a prefetch.
But if we chain to the tail element, considering there are two elements in the 
queue now. There is prefetching skipped during nextRows() iteration:
Deferred(no prefetching), Deferred(prefetching)



--
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: 6
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 15:28:42 +0000
Gerrit-HasComments: Yes

Reply via email to