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

Change subject: KUDU-2525: KuduTableInputFormat may halt before exhausting scan
......................................................................


Patch Set 2:

(2 comments)

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
> I can be sure that there is not an empty tablet in the table.
Could you try to synthesize this into a unit test? That'll help solidify the 
conditions that must be met for this to occur, and will also serve as a 
regression test.


http://gerrit.cloudera.org:8080/#/c/11111/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java@448
PS1, Line 448:         // TODO(qqzhang) scanner.nextRows() sometimes returns an 
empty RowResultIterator,
> Done
Thanks for the added content, but why TODO(...)? Again, we only use TODOs when 
there's some work left to be done.

See this Google Style Guide link for more details: 
https://google.github.io/styleguide/cppguide.html#TODO_Comments



--
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: 2
Gerrit-Owner: zhangqianqiong <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhangqianqiong <[email protected]>
Gerrit-Comment-Date: Fri, 03 Aug 2018 16:49:02 +0000
Gerrit-HasComments: Yes

Reply via email to