Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/14958 )
Change subject: [tools] Add 'run_cleanup' option for 'kudu perf loadgen' ...................................................................... Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/14958/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14958/2//COMMIT_MSG@9 PS2, Line 9: Add 'run_cleanup' option to provide a way to cleanup test data written to : the table, especially an existing user table. > I guess if cleanup isn't on by default it's OK with me. Done http://gerrit.cloudera.org:8080/#/c/14958/2/src/kudu/tools/tool_action_perf.cc File src/kudu/tools/tool_action_perf.cc: http://gerrit.cloudera.org:8080/#/c/14958/2/src/kudu/tools/tool_action_perf.cc@307 PS2, Line 307: DEFINE_bool(run_cleanup, true, > +1 to Adar's comment: I also think this should be `false` by default at lea Done http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc File src/kudu/tools/tool_action_perf.cc: http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@464 PS3, Line 464: value_gen.reset(new Generator(*key_gen)); > This is somewhat expensive, as it's being created per-row rather than per-t Done http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@498 PS3, Line 498: kMaxUnscaledDecimal32))); > Nit: fix the indentation here? Done http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@650 PS3, Line 650: operate rows > Nit: maybe "rows total"? Done http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@665 PS3, Line 665: if (!status.ok() || total_err_count != 0) { > Indentation in this block is off. Done http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@730 PS3, Line 730: >= > Why this change? In an unit test, I want to create a single tablet by setting table_num_hash_partitions and table_num_range_partitions to 1, then we can easily check sequential rows in the tablet. And I think we don't need to keep this restriction. -- To view, visit http://gerrit.cloudera.org:8080/14958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e75adde434bac5e88151361655526b91f327b4c Gerrit-Change-Number: 14958 Gerrit-PatchSet: 3 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Sun, 12 Jan 2020 08:24:41 +0000 Gerrit-HasComments: Yes
