You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2018/07/30 22:44:09 UTC
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Hello Alexey Serbin, Hao Hao,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/11084
to review the following change.
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
perf loadgen: add --auto_database flag and change auto table name generation
This commit changes the 'perf loadgen' tool to create HMS-compliant
tables when an auto table is created. A new flag allows the database to
be configured, and defaults to the 'default' HMS database.
Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_perf.cc
2 files changed, 22 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/11084/1
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 5: Verified+1
unrelated flake
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 19:06:41 +0000
Gerrit-HasComments: No
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 6:
(2 comments)
Sorry, I was a little trigger happy with the submit button!
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG@10
PS2, Line 10: A new flag allows the database to
: be configured, and defaults to the 'default' HMS database.
> Do you think we should default it to empty string, since hms integration is
The downside of that is that it becomes a de-facto required option when working with a cluster where the HMS integration is enabled, or if you don't know if the HMS integration is disabled. For that reason I think changing the auto-table name format will be easier to deal with for end users than adding a required flag.
http://gerrit.cloudera.org:8080/#/c/11084/5/src/kudu/tools/tool_action_perf.cc
File src/kudu/tools/tool_action_perf.cc:
http://gerrit.cloudera.org:8080/#/c/11084/5/src/kudu/tools/tool_action_perf.cc@294
PS5, Line 294: a table is "
: "not created
> nit: 'existing table is used' to be more clear?
Yes, I'll change that in a follow-up commit.
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 19:16:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG@10
PS2, Line 10: A new flag allows the database to
: be configured, and defaults to the 'default' HMS database.
> In a few cases I've modified tests to use an HMS-compatible name even thoug
Should we be more coy about it, to avoid inflating user expectations vis a vis database support when HMS integration is not being used? For example, we could just advertise the flag as "prefix to include before a dot and the loadgen table name" rather than "database".
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 00:14:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11084
to look at the new patch set (#4).
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
perf loadgen: add --auto_database flag and change auto table name generation
This commit changes the 'perf loadgen' tool to create HMS-compliant
tables when an auto table is created. A new flag allows the database to
be configured, and defaults to the 'default' HMS database.
Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_perf.cc
2 files changed, 27 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/11084/4
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has removed a vote on this change.
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11084
to look at the new patch set (#2).
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
perf loadgen: add --auto_database flag and change auto table name generation
This commit changes the 'perf loadgen' tool to create HMS-compliant
tables when an auto table is created. A new flag allows the database to
be configured, and defaults to the 'default' HMS database.
Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_perf.cc
2 files changed, 22 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/11084/2
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 17:22:28 +0000
Gerrit-HasComments: No
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
perf loadgen: add --auto_database flag and change auto table name generation
This commit changes the 'perf loadgen' tool to create HMS-compliant
tables when an auto table is created. A new flag allows the database to
be configured, and defaults to the 'default' HMS database.
Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Reviewed-on: http://gerrit.cloudera.org:8080/11084
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Dan Burkert <da...@apache.org>
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_perf.cc
2 files changed, 27 insertions(+), 1 deletion(-)
Approvals:
Adar Dembo: Looks good to me, approved
Alexey Serbin: Looks good to me, approved
Dan Burkert: Verified
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 1:
(2 comments)
lgtm, just a couple of formatting nits
http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1504
PS1, Line 1504:
nit: is this spacing correct and intended?
http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1514
PS1, Line 1514:
nit: the same question on the spacing
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Jul 2018 23:23:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Jul 2018 23:50:06 +0000
Gerrit-HasComments: No
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG@10
PS2, Line 10: A new flag allows the database to
: be configured, and defaults to the 'default' HMS database.
> This sort-of kind-of exposes the idea of a database beyond just the confine
In a few cases I've modified tests to use an HMS-compatible name even though the test only runs with HMS integration enabled via a parameter. This is the first and only non-test code to my knowledge. That follows pretty directly from this being the only non-test code which creates tables.
If we wanted to be really fancy we could auto-detect whether the HMS is enabled through the GetFlags API and do something special, but in my opinion it's better to change the behavior now and make it consistent going forward.
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 00:09:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG@10
PS2, Line 10: A new flag allows the database to
: be configured, and defaults to the 'default' HMS database.
> I don't think being coy will help, but I've added a sentence about being us
Do you think we should default it to empty string, since hms integration is not enabled by default?
http://gerrit.cloudera.org:8080/#/c/11084/5/src/kudu/tools/tool_action_perf.cc
File src/kudu/tools/tool_action_perf.cc:
http://gerrit.cloudera.org:8080/#/c/11084/5/src/kudu/tools/tool_action_perf.cc@294
PS5, Line 294: a table is "
: "not created
nit: 'existing table is used' to be more clear?
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 19:08:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1504
PS1, Line 1504:
> nit: is this spacing correct and intended?
It's what my editor did automatically, it's two spaces per open paren. Do we have a standard for this?
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Jul 2018 23:24:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG@10
PS2, Line 10: A new flag allows the database to
: be configured, and defaults to the 'default' HMS database.
> Should we be more coy about it, to avoid inflating user expectations vis a
I don't think being coy will help, but I've added a sentence about being useful primarily for the HMS integration. I've also added an option to restore the previous behavior by setting the flag to the empty string.
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 00:28:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1504
PS1, Line 1504: Host
> Well, I'm not sure about standard, but I thought it would be either 4 space
Done
http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1514
PS1, Line 1514: Host
> nit: the same question on the spacing
Done
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Jul 2018 23:31:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 6:
Sorry for being late in the code review. Just realized it is already committed. My comments are mainly some nites and please ignore it if you feel it is not relevant/important.
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 19:13:10 +0000
Gerrit-HasComments: No
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1504
PS1, Line 1504:
> It's what my editor did automatically, it's two spaces per open paren. Do
Well, I'm not sure about standard, but I thought it would be either 4 spaces (i.e. twice the embedded scope shift) or aligned with the opening corresponding opening parenthesis. But it's just a nit, though.
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Jul 2018 23:27:16 +0000
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 18:42:49 +0000
Gerrit-HasComments: No
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 )
Change subject: perf loadgen: add --auto_database flag and change auto table name generation
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG@10
PS2, Line 10: A new flag allows the database to
: be configured, and defaults to the 'default' HMS database.
This sort-of kind-of exposes the idea of a database beyond just the confines of an HMS integrated Kudu. Is that something we've already done in past HMS integration patches?
--
To view, visit http://gerrit.cloudera.org:8080/11084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a
Gerrit-Change-Number: 11084
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Jul 2018 23:54:32 +0000
Gerrit-HasComments: Yes