Adar Dembo 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. > 'perf loadgen' data is written on existing columns, not associate with the I guess if cleanup isn't on by default it's OK with me. 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, > This means cleanup new generated dirty data by default. Yeah, but why does it deserve to default to on? That could surprise users who expect the current behavior (where inserted rows remain inserted). 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-thread. Is there a way we could avoid it while still faithfully replicating whatever PRNG state we need from the key generator? 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? 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"? 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. http://gerrit.cloudera.org:8080/#/c/14958/3/src/kudu/tools/tool_action_perf.cc@730 PS3, Line 730: >= Why this change? -- 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 <405403...@qq.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Mon, 06 Jan 2020 22:11:48 +0000 Gerrit-HasComments: Yes
