Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11111 )

Change subject: KUDU-2525: fix a bug for kudu: scanner return the zero rows
......................................................................


Patch Set 1:

(3 comments)

Could you reproduce the bug in a kudu-mapreduce unit test?

http://gerrit.cloudera.org:8080/#/c/11111/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11111/1//COMMIT_MSG@7
PS1, Line 7: KUDU-2525: fix a bug for kudu: scanner return the zero rows
Rewrite as:

  KUDU-2525: KuduTableInputFormat may halt before exhausting scan


http://gerrit.cloudera.org:8080/#/c/11111/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java
File 
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java:

http://gerrit.cloudera.org:8080/#/c/11111/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java@66
PS1, Line 66:  * This input format generates one split per tablet and the only 
location for each split is that
Given that there's one split per tablet, how does this bug manifest? I thought 
the only time scanner.nextRows() returned an empty RowResultIterator was when 
the scan ended on one tablet and started up on the next, and the next tablet 
didn't have any rows.


http://gerrit.cloudera.org:8080/#/c/11111/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java@448
PS1, Line 448:         // FIXME(qqzhang) the loop is only for the responded 
empty rows
Typically we mark things like this with TODO(...); what's left to do here?

Could you instead modify this comment to make it more instructive? You can 
explain how scanner.nextRows() is allowed to return an empty RowResultIterator, 
but until scanner.hasMoreRows() returns false, we need to continue iterating on 
the scanner.



--
To view, visit http://gerrit.cloudera.org:8080/11111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbfdd2efbd281e4d849917664b33e183e180bafd
Gerrit-Change-Number: 11111
Gerrit-PatchSet: 1
Gerrit-Owner: zhangqianqiong <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 02 Aug 2018 18:07:14 +0000
Gerrit-HasComments: Yes

Reply via email to