You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2019/05/23 05:38:51 UTC
[kudu-CR] hms: followup to 6a8d1d2
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13410
Change subject: hms: followup to 6a8d1d2
......................................................................
hms: followup to 6a8d1d2
Commit 6a8d1d2 left room for a little bit of refactoring and testing.
This patch fills this room:
- minor refactoring in tool_action_hms
- tested that altering and dropping external tables won't affect
underlying Kudu data
There are no functional changes in this patch.
Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
---
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/hms_itest-base.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/tools/tool_action_hms.cc
4 files changed, 55 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/13410/1
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 24 May 2019 00:47:50 +0000
Gerrit-HasComments: No
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/tools/tool_action_hms.cc@328
PS2, Line 328: managed_hms_tables_by_id[(*kudu_table)->id()].emplace_back(
> nit: it's highly unlikely that another type of Kudu table appears in HMS,
I don't think we should DCHECK because that's a property of the HMS and not in Kudu; also ISTR there being other types in Hive (e.g. managed_views, indexed_tables here https://hive.apache.org/javadocs/r2.1.1/api/org/apache/hadoop/hive/metastore/TableType.html), though we don't expect them in Kudu.
I explicitly handled managed here instead.
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 20:36:58 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/integration-tests/master_hms-itest.cc@409
PS2, Line 409: ASSERT_OK(hms_client_->AlterTable("default", "ext2", new_ext));
> External references will break when we rename the underlying table. That's
Thank you for the clarification. It's seems I need to re-read the doc on the external tables proposal :(
BTW, do you think it's worth adding a bit of code in this case to explicitly 'document' that external references will break in that case?
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 18:07:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/integration-tests/master_hms-itest.cc@409
PS2, Line 409: ASSERT_OK(hms_client_->AlterTable("default", "ext2", new_ext));
Maybe, add hms_client_->GetTable("default", "ext2", ...) and hms_client_->GetTable("default", "ext3", ...), making sure the former fails and the latter succeeds? Not sure whether that's already guaranteed to be like this after AlterTable() completed successfully.
Also, do we have a test to make sure the external references work as expected once we rename the underlying managed table?
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 17:56:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/integration-tests/master_hms-itest.cc@409
PS2, Line 409: ASSERT_OK(hms_client_->AlterTable("default", "ext2", new_ext));
> Maybe, add hms_client_->GetTable("default", "ext2", ...) and hms_client_->G
External references will break when we rename the underlying table. That's something we can decide how we want to handle. Logging as a warning is probably the minimum handling. But auto-repointing might not be something we should do.
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 18:00:41 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 23:34:38 +0000
Gerrit-HasComments: No
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/tools/tool_action_hms.cc@328
PS2, Line 328: managed_hms_tables_by_id[(*kudu_table)->id()].emplace_back(
> Right, but this is a Kudu tool -- we'd hit that check if we were to run thi
Yes, you ignore that and put it into managed_hms_tables_by_id, and that's considered Kudu table with ID. If that's not a problem for the tool, I think that's tolerable. But the idea was to add a DCHECK to spot such misconfigurations only in debug mode.
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 24 May 2019 18:40:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13410
to look at the new patch set (#5).
Change subject: hms: followup to 6a8d1d2
......................................................................
hms: followup to 6a8d1d2
Commit 6a8d1d2 left room for a little bit of refactoring and testing.
This patch fills this room:
- minor refactoring in tool_action_hms
- tested that altering and dropping external tables won't affect
underlying Kudu data
There are no functional changes in this patch.
Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
---
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/hms_itest-base.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/tools/tool_action_hms.cc
4 files changed, 66 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/13410/5
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/1/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/13410/1/src/kudu/tools/tool_action_hms.cc@307
PS1, Line 307: // If this is a non-legacy, managed table, we expect a table ID to exist
> Can you adjust this to match the logic below more closely. Something like:
Done
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 17:33:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/tools/tool_action_hms.cc@328
PS2, Line 328: managed_hms_tables_by_id[(*kudu_table)->id()].emplace_back(
nit: it's highly unlikely that another type of Kudu table appears in HMS, but maybe add DCHECK_EQ(HmsClient::kManagedTable, hms_table.tableType) anyways. Feel free to ignore if this sounds too paranoid.
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 18:43:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
hms: followup to 6a8d1d2
Commit 6a8d1d2 left room for a little bit of refactoring and testing.
This patch fills this room:
- minor refactoring in tool_action_hms
- tested that altering and dropping external tables won't affect
underlying Kudu data
There are no functional changes in this patch.
Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Reviewed-on: http://gerrit.cloudera.org:8080/13410
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Hao Hao <ha...@cloudera.com>
---
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/hms_itest-base.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/tools/tool_action_hms.cc
4 files changed, 66 insertions(+), 29 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, but someone else must approve
Andrew Wong: Verified
Hao Hao: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/tools/tool_action_hms.cc@328
PS2, Line 328: managed_hms_tables_by_id[(*kudu_table)->id()].emplace_back(
> I don't think we should DCHECK because that's a property of the HMS and not
Right, since we don't expect them in Kudu, I meant adding DCHECK_EQ() at line 328 of PS2 of this file.
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 23:19:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/integration-tests/master_hms-itest.cc@409
PS2, Line 409: ASSERT_OK(hms_client_->AlterTable("default", "ext2", new_ext));
> Thank you for the clarification. It's seems I need to re-read the doc on t
Done
Makes sense, added some checking for that.
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 18:22:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 18:37:21 +0000
Gerrit-HasComments: No
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 21:11:23 +0000
Gerrit-HasComments: No
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 5: Verified+1
(1 comment)
All unrelated java flaky tests. I think Adar has been working to fix the Java infra.
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/tools/tool_action_hms.cc@328
PS2, Line 328: managed_hms_tables_by_id[(*kudu_table)->id()].emplace_back(
> Right, since we don't expect them in Kudu, I meant adding DCHECK_EQ() at li
Right, but this is a Kudu tool -- we'd hit that check if we were to run this tool and that table type happened to be in the HMS, when we should _really_ just ignore it or catch it and warn the user. I opted to ignore it.
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 23:39:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 18:07:15 +0000
Gerrit-HasComments: No
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/13410/2/src/kudu/integration-tests/master_hms-itest.cc@409
PS2, Line 409: ASSERT_OK(hms_client_->AlterTable("default", "ext2", ext));
> Thank you for the clarification. It's seems I need to re-read the doc on t
I think we can 'document' that in the tooling, but as we don't synchronize external tables, I don't see why we want to do that in Kudu master.
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 18:27:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13410
to look at the new patch set (#3).
Change subject: hms: followup to 6a8d1d2
......................................................................
hms: followup to 6a8d1d2
Commit 6a8d1d2 left room for a little bit of refactoring and testing.
This patch fills this room:
- minor refactoring in tool_action_hms
- tested that altering and dropping external tables won't affect
underlying Kudu data
There are no functional changes in this patch.
Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
---
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/hms_itest-base.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/tools/tool_action_hms.cc
4 files changed, 63 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/13410/3
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: hms: followup to 6a8d1d2
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13410
to look at the new patch set (#2).
Change subject: hms: followup to 6a8d1d2
......................................................................
hms: followup to 6a8d1d2
Commit 6a8d1d2 left room for a little bit of refactoring and testing.
This patch fills this room:
- minor refactoring in tool_action_hms
- tested that altering and dropping external tables won't affect
underlying Kudu data
There are no functional changes in this patch.
Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
---
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/hms_itest-base.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/tools/tool_action_hms.cc
4 files changed, 55 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/13410/2
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13410
to look at the new patch set (#4).
Change subject: hms: followup to 6a8d1d2
......................................................................
hms: followup to 6a8d1d2
Commit 6a8d1d2 left room for a little bit of refactoring and testing.
This patch fills this room:
- minor refactoring in tool_action_hms
- tested that altering and dropping external tables won't affect
underlying Kudu data
There are no functional changes in this patch.
Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
---
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/hms_itest-base.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/tools/tool_action_hms.cc
4 files changed, 65 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/13410/4
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13410/1/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/13410/1/src/kudu/tools/tool_action_hms.cc@307
PS1, Line 307: // If this is a legacy table or external table, we don't expect a table
Can you adjust this to match the logic below more closely. Something like:
> If this is a managed Kudu table, lookup the table by ID. Otherwise look it up by table name.
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 13:47:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: followup to 6a8d1d2
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13410 )
Change subject: hms: followup to 6a8d1d2
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13410
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
Gerrit-Change-Number: 13410
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 23 May 2019 17:53:49 +0000
Gerrit-HasComments: No