Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10633 )

Change subject: KUDU-1861: add range-partitions to loadgen tables
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10633/5/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/10633/5/src/kudu/tools/kudu-tool-test.cc@1487
PS5, Line 1487:
              :     // Keep the tab
> Actually I did mean to use `use_random` to make sure nothing break _even if
Thank you for the explanation.  Could you add those details into the comment 
itself?  I think it would be clearer for the reader than '... nothing funny 
happens ...'.


http://gerrit.cloudera.org:8080/#/c/10633/5/src/kudu/tools/kudu-tool-test.cc@1512
PS5, Line 1512:   args.emplace_back("--table_num_range_partitions=2");
              :   args.emplace_back("--table_num_hash_partitio
> We're not deleting the tables as we go so we can do this check. That also m
I'm not sure I understand.  My comment was about reducing

ASSERT_OK(WaitForNumTabletsOnTS(ts, expected_tablets, kTimeout, &tablets));
ASSERT_EQ(expected_tablets, tablets.size());

to

SSERT_OK(WaitForNumTabletsOnTS(ts, expected_tablets, kTimeout));

The idea is that you don't need the second assert because of the way how 
WaitForNumTabletsOnTS() works.


http://gerrit.cloudera.org:8080/#/c/10633/6/src/kudu/tools/tool_action_perf.cc
File src/kudu/tools/tool_action_perf.cc:

http://gerrit.cloudera.org:8080/#/c/10633/6/src/kudu/tools/tool_action_perf.cc@325
PS6, Line 325: static
nit: why do you need the 'static' specifier for the function within an 
anonymous namespace?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4f552a4c73dc82f3b7934082769522557ee5013
Gerrit-Change-Number: 10633
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Sat, 23 Jun 2018 04:14:07 +0000
Gerrit-HasComments: Yes

Reply via email to