Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16253 )
Change subject: [loadgen] Add separate flags to populate random values for PK and non-PK cols ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/16253/4//COMMIT_MSG Commit Message: PS4: nit: Do you min keeping the lines of the commit description under 72 characters long, when possible? That would be in-line with https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines mentioned at https://kudu.apache.org/docs/contributing.html#_submitting_patches http://gerrit.cloudera.org:8080/#/c/16253/4//COMMIT_MSG@14 PS4, Line 14: 2 nit: two http://gerrit.cloudera.org:8080/#/c/16253/4/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/16253/4/src/kudu/tools/kudu-tool-test.cc@2268 PS4, Line 2268: // Test to ensure if both '--use_random' and '--use_random_non_pk'/'--use_random_pk' are : // specified then '--use_random' is ignored. : NO_FATALS(RunLoadgen(5, : { : // Large number of rows are okay since only non-pk columns will use random values and : // the deprecated '--use_random' will be ignored when used with '--use_random_non_pk'. : "--num_rows_per_thread=4096", : "--num_threads=2", : "--run_scan", : "--string_len=8", : "--use_random", : "--use_random_non_pk", : }, : "bench_auto_flush_background_random_deprecated")); It seems current code assumes there is just a single call of RunLoadgen() per test scenario, otherwise a memory leak happens at https://github.com/apache/kudu/blob/master/src/kudu/integration-tests/cluster_itest_util.cc#L309 called from https://github.com/apache/kudu/blob/master/src/kudu/tools/kudu-tool-test.cc#L913 I guess that's a separate issue, but in this changelist I would simply put those calls under separate test scenarios. -- To view, visit http://gerrit.cloudera.org:8080/16253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35bb33a192baa9e6e67e85bcbd5ca7164ba154e4 Gerrit-Change-Number: 16253 Gerrit-PatchSet: 4 Gerrit-Owner: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 31 Jul 2020 00:40:32 +0000 Gerrit-HasComments: Yes
