Alexey Serbin has posted comments on this change.

Change subject: [tools] added insert-generated-rows into kudu tools

Patch Set 16:

File src/kudu/tools/

Line 180: TEST_F(ToolTest, TestTopLevelHelp) {
> Can you update this test with the new mode and action?

PS16, Line 781: client::sp::
> Nit: you added a "using" for this at the top of the file.

PS16, Line 788: vector<string>(
> Why is this needed?
Because otherwise it will be an error like

Bad status: Invalid argument: Table partitioning must be specified using 
add_hash_partitions or set_range_partition_columns

PS16, Line 789: wait(true)
> Can omit, this is the default behavior.

PS16, Line 798: ASSERT_OK(cluster->Start());
> You can move this into LoadgenCreateMiniCluster if you want. You could even
Good idea.  For some reason, I thought keeping separate functions would be 
better, but apparently there isn't any demand for that.  Will update.

PS16, Line 819: string("-table_name=") + kTableName
> Nit: Substitute("-table_name=%s", kTableName) is more idiomatic.

Line 820:     "-buffer_flush_watermark_pct=0.125",
> Nit: can you use -- to prefix gflags, for consistency with existing tests?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Alexey Serbin <>
Gerrit-Reviewer: David Ribeiro Alves <>
Gerrit-Reviewer: Dinesh Bhat <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to