[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/15681 )
Change subject: KUDU-3078 Add Ranger tests to master_authz-itest
..
Patch Set 13: Code-Review+1
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15681/13/src/kudu/integration-tests/master_authz-itest.cc
File src/kudu/integration-tests/master_authz-itest.cc:
http://gerrit.cloudera.org:8080/#/c/15681/13/src/kudu/integration-tests/master_authz-itest.cc@515
PS13, Line 515: policy.tables.emplace_back("*");
Mentioned in previous comments, I think this can be removed.
http://gerrit.cloudera.org:8080/#/c/15681/13/src/kudu/integration-tests/master_authz-itest.cc@554
PS13, Line 554: policy_new_table.tables.emplace_back("*");
This can be removed as well.
http://gerrit.cloudera.org:8080/#/c/15681/13/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15681/13/src/kudu/ranger/ranger_client.cc@573
PS13, Line 573: if (allowed_actions.empty()) {
: LOG(WARNING) << Substitute("User $0 is not authorized to
perform actions $1 on table $2",
:user_name, JoinMapped(*actions,
ActionPB_Name, ", "), table_name);
: return Status::NotAuthorized(kUnauthorizedAction);
: }
Should we remove this and let ranger_authz_provider handle it as well?
--
To view, visit http://gerrit.cloudera.org:8080/15681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf
Gerrit-Change-Number: 15681
Gerrit-PatchSet: 13
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 09 Apr 2020 05:52:14 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3078 Refactor Sentry integration tests
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15671 ) Change subject: KUDU-3078 Refactor Sentry integration tests .. Patch Set 8: I think it's worth squashing this into the next patch -- without seeing how the harness is used, it's pretty hard to review in isolation. -- To view, visit http://gerrit.cloudera.org:8080/15671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieff25eaf7a41cf7d8fe965738907e67bbd9b9051 Gerrit-Change-Number: 15671 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 09 Apr 2020 05:18:04 + Gerrit-HasComments: No
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Andrew Wong has removed a vote on this change. Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15681 ) Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. Patch Set 13: Verified+1 Unrelated flake: BlockManagerType/TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 09 Apr 2020 05:07:49 + Gerrit-HasComments: No
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/15681 )
Change subject: KUDU-3078 Add Ranger tests to master_authz-itest
..
Patch Set 13:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/15681/1/src/kudu/integration-tests/master_authz-itest.cc
File src/kudu/integration-tests/master_authz-itest.cc:
http://gerrit.cloudera.org:8080/#/c/15681/1/src/kudu/integration-tests/master_authz-itest.cc@1151
PS1, Line 1151: zITestBase::SetUp());
> How do you feel about adding a TODO to refactor it once we support C++14? W
The latest patchset seems to be working with the approach described here.
http://gerrit.cloudera.org:8080/#/c/15681/7/src/kudu/integration-tests/master_authz-itest.cc
File src/kudu/integration-tests/master_authz-itest.cc:
http://gerrit.cloudera.org:8080/#/c/15681/7/src/kudu/integration-tests/master_authz-itest.cc@540
PS7, Line 540:
> We should use ParseRangerTableIdentifier?
Done
http://gerrit.cloudera.org:8080/#/c/15681/9/src/kudu/integration-tests/master_authz-itest.cc
File src/kudu/integration-tests/master_authz-itest.cc:
http://gerrit.cloudera.org:8080/#/c/15681/9/src/kudu/integration-tests/master_authz-itest.cc@245
PS9, Line 245:
> warning: method 'CreateKuduTable' can be made static [readability-convert-m
Done
http://gerrit.cloudera.org:8080/#/c/15681/9/src/kudu/integration-tests/master_authz-itest.cc@285
PS9, Line 285: KuduSchema schema;
> warning: method 'CheckTable' can be made static [readability-convert-member
Done
http://gerrit.cloudera.org:8080/#/c/15681/9/src/kudu/integration-tests/master_authz-itest.cc@287
PS9, Line 287: unique_ptr
table_creator(client->NewTableCreator());
> warning: parameter 'user' is unused [misc-unused-parameters]
Done
http://gerrit.cloudera.org:8080/#/c/15681/9/src/kudu/integration-tests/master_authz-itest.cc@288
PS9, Line 288: if (timeout.Initialized()) {
> warning: parameter 'cluster' is unused [misc-unused-parameters]
Done
http://gerrit.cloudera.org:8080/#/c/15681/9/src/kudu/integration-tests/master_authz-itest.cc@568
PS9, Line 568: AuthorizationPolicy policy;
> warning: parameter 'cluster' is unused [misc-unused-parameters]
Done
http://gerrit.cloudera.org:8080/#/c/15681/9/src/kudu/integration-tests/master_authz-itest.cc@572
PS9, Line 572: SleepFor(MonoDelta::FromMilliseconds(1500));
> warning: parameter 'cluster' is unused [misc-unused-parameters]
Done
--
To view, visit http://gerrit.cloudera.org:8080/15681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf
Gerrit-Change-Number: 15681
Gerrit-PatchSet: 13
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 09 Apr 2020 04:58:08 +
Gerrit-HasComments: Yes
[kudu-CR] net: make Sockaddr more generic for other address families
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15690 ) Change subject: net: make Sockaddr more generic for other address families .. Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.h File src/kudu/util/net/sockaddr.h: http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.h@59 PS2, Line 59: // Compare two addresses bytewise. : static bool BytewiseLess(const Sockaddr& a, const Sockaddr& b); : : // Return the IPv4 wildcard address. : static Sockaddr Wildcard(); style nit: move these two static methods up to be in the very beginning of the method list? http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.h@81 PS2, Line 81: int nit: while you are here, maybe change this to be uint16_t ? http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.h@85 PS2, Line 85: int ditto http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.cc File src/kudu/util/net/sockaddr.cc: http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.cc@108 PS2, Line 108: addrlen Might it be the case that other.addrlen() != addrlen() ? http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.cc@173 PS2, Line 173: ditto http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.cc@173 PS2, Line 173: style nit: remove the extra space http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.cc@173 PS2, Line 173: const_cast I might be missing something, but it seems const_cast is not needed? http://man7.org/linux/man-pages/man3/getnameinfo.3.html -- To view, visit http://gerrit.cloudera.org:8080/15690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifff57e13a0e9b86b191d776ead67371b1e2ed4e3 Gerrit-Change-Number: 15690 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Thu, 09 Apr 2020 04:45:22 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Andrew Wong has uploaded a new patch set (#13) to the change originally created by Attila Bukor. ( http://gerrit.cloudera.org:8080/15681 ) Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. KUDU-3078 Add Ranger tests to master_authz-itest master_sentry-itest is renamed to master_authz-itest to generalize it, and most tests are changed to also run with Ranger. Those that aren't test behavior specific to Sentry. A later patch will do the same for ts_sentry-itest. Ranger doesn't support adding new policy items (users and privileges) to existing policies, so MiniRanger::AddPolicy stores policies in a member variable and recreates them completely when a new item is added to an existing policy (resource). Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf --- M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/hms_itest-base.h R src/kudu/integration-tests/master_authz-itest.cc M src/kudu/integration-tests/master_hms-itest.cc M src/kudu/master/ranger_authz_provider.cc M src/kudu/ranger/mini_ranger-test.cc M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/ranger/ranger_client.h 11 files changed, 832 insertions(+), 558 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15681/13 -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Andrew Wong has uploaded a new patch set (#12) to the change originally created by Attila Bukor. ( http://gerrit.cloudera.org:8080/15681 ) Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. KUDU-3078 Add Ranger tests to master_authz-itest master_sentry-itest is renamed to master_authz-itest to generalize it, and most tests are changed to also run with Ranger. Those that aren't test behavior specific to Sentry. A later patch will do the same for ts_sentry-itest. Ranger doesn't support adding new policy items (users and privileges) to existing policies, so MiniRanger::AddPolicy stores policies in a member variable and recreates them completely when a new item is added to an existing policy (resource). Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf --- M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/master_authz-itest.cc M src/kudu/master/ranger_authz_provider.cc M src/kudu/ranger/mini_ranger-test.cc M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/ranger/ranger_client.h 9 files changed, 828 insertions(+), 554 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15681/12 -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 12 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. encoding-test: Clean up bitshuffle tests a little Also adds functions in random number generator to generate 128-bit integers. Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Reviewed-on: http://gerrit.cloudera.org:8080/15043 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins --- M src/kudu/cfile/encoding-test.cc M src/kudu/util/random.h M src/kudu/util/random_util-test.cc M src/kudu/util/random_util.h 4 files changed, 158 insertions(+), 34 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 13 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] net: make Sockaddr more generic for other address families
Mike Percy has posted comments on this change. (
http://gerrit.cloudera.org:8080/15690 )
Change subject: net: make Sockaddr more generic for other address families
..
Patch Set 2: Code-Review+2
(1 comment)
Looks like a step in the right direction for this API.
http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.h
File src/kudu/util/net/sockaddr.h:
http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.h@131
PS2, Line 131: union {
Clearly sockaddr_storage was designed to be used in this way but I never knew
about this. Very nice.
--
To view, visit http://gerrit.cloudera.org:8080/15690
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifff57e13a0e9b86b191d776ead67371b1e2ed4e3
Gerrit-Change-Number: 15690
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Volodymyr Verovkin
Gerrit-Comment-Date: Thu, 09 Apr 2020 03:58:07 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Andrew Wong has uploaded a new patch set (#11) to the change originally created by Attila Bukor. ( http://gerrit.cloudera.org:8080/15681 ) Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. KUDU-3078 Add Ranger tests to master_authz-itest master_sentry-itest is renamed to master_authz-itest to generalize it and the tests were changed to also run with Ranger. A later patch will do the same for ts_sentry-itest. Ranger doesn't support adding new policy items (users and privileges) to existing policies, so MiniRanger::AddPolicy stores policies in a member variable and recreates them completely when a new item is added to an existing policy (resource). Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf --- M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/master_authz-itest.cc M src/kudu/master/ranger_authz_provider.cc M src/kudu/ranger/mini_ranger-test.cc M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/ranger/ranger_client.h 9 files changed, 736 insertions(+), 466 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15681/11 -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 11 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. Patch Set 12: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h File src/kudu/util/random_util.h: http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h@146 PS9, Line 146: dom(max_v > Done Thanks! -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 12 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Apr 2020 01:49:52 + Gerrit-HasComments: Yes
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h File src/kudu/util/random_util.h: http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h@146 PS9, Line 146: dom(max_v > It seems convenient to place it where the suppression is, especially for su Done -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 12 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Apr 2020 01:40:24 + Gerrit-HasComments: Yes
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Bankim Bhavsar has uploaded a new patch set (#12) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. encoding-test: Clean up bitshuffle tests a little Also adds functions in random number generator to generate 128-bit integers. Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 --- M src/kudu/cfile/encoding-test.cc M src/kudu/util/random.h M src/kudu/util/random_util-test.cc M src/kudu/util/random_util.h 4 files changed, 158 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/43/15043/12 -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 12 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15042 ) Change subject: cfile: change BlockBuilder API to yield a vector of Slices .. cfile: change BlockBuilder API to yield a vector of Slices When blocks are appended to cfiles at the IO layer, we already have the ability to write multiple slices using a vectored IO. Prior to this patch, the BlockBuilder API was restricted to returning a single slice, whereas it would be more convenient in some cases to be able to return multiple slices (eg separating the header from the data). This new functionality is used by BinaryDictBlockBuilder to avoid an extra copy in Finish(). Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Reviewed-on: http://gerrit.cloudera.org:8080/15042 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins --- M src/kudu/cfile/binary_dict_block.cc M src/kudu/cfile/binary_dict_block.h M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/binary_plain_block.h M src/kudu/cfile/binary_prefix_block.cc M src/kudu/cfile/binary_prefix_block.h M src/kudu/cfile/block_encodings.h M src/kudu/cfile/bshuf_block.cc M src/kudu/cfile/bshuf_block.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/encoding-test.cc M src/kudu/cfile/plain_bitmap_block.h M src/kudu/cfile/plain_block.h M src/kudu/cfile/rle_block.h 14 files changed, 115 insertions(+), 107 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/15042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Gerrit-Change-Number: 15042 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] cfile: clean up encoding-test to use fewer templates
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15044 ) Change subject: cfile: clean up encoding-test to use fewer templates .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15044 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iba4464c2ea41107df96c68ea61576a0ea269277a Gerrit-Change-Number: 15044 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Apr 2020 01:21:52 + Gerrit-HasComments: No
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h File src/kudu/util/random_util.h: http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h@146 PS9, Line 146: exception > I think comment is better placed close to the line that raises ASAN/UBSAN w It seems convenient to place it where the suppression is, especially for such a small function. When I read the code here, I'm thinking more about random number generation than I am about ASAN, so this comment seems a little out of place, at least without more context. Maybe be a bit more explicit here then: "Hence the ASAN suppression for this template" or somesuch. -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 11 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Apr 2020 01:21:32 + Gerrit-HasComments: Yes
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Bankim Bhavsar has posted comments on this change. (
http://gerrit.cloudera.org:8080/15043 )
Change subject: encoding-test: Clean up bitshuffle tests a little
..
Patch Set 11:
> Patch Set 11: Verified-1
>
> Build Failed
>
> http://jenkins.kudu.apache.org/job/kudu-gerrit/21379/ : FAILURE
Unrelated test failures.
TSAN Java test failure is due to failure to start chronyd.
00:57:38.307 [INFO - cluster stderr printer] (MiniKuduCluster.java:590) W0409
00:57:38.235510 345 mini_chronyd.cc:189] Time spent starting chronyd: real
1.028s user 0.015s sys 0.038s
00:57:38.350 [DEBUG - main] (MiniKuduCluster.java:176) Response: error {
code: TIMED_OUT
message: "failed to start NTP server 0: failed to contact chronyd in 1.000s"
}
RELEASE build failure is due to some network error for python.
17:47:41 Downloading
https://files.pythonhosted.org/packages/05/d4/72eaba30abdcee0bb99cbdb21dbdb3f5d23a5041574fa7d94003b9afd3bc/numpy-1.15.4-cp34-cp34m-manylinux1_x86_64.whl
(13.8MB)
17:47:41 Exception:
17:47:41 Traceback (most recent call last):
17:47:41 File
"/home/jenkins-slave/workspace/kudu-master/1/build/release/py_env/lib/python3.4/site-packages/pip/_vendor/urllib3/response.py",
line 331, in _error_catcher
17:47:41 yield
17:47:41 File
"/home/jenkins-slave/workspace/kudu-master/1/build/release/py_env/lib/python3.4/site-packages/pip/_vendor/urllib3/response.py",
line 413, in read
17:47:41 data = self._fp.read(amt)
17:47:41 File "/usr/lib/python3.4/http/client.py", line 529, in read
17:47:41 return super(HTTPResponse, self).read(amt)
17:47:41 File "/usr/lib/python3.4/http/client.py", line 568, in readinto
17:47:41 n = self.fp.readinto(b)
17:47:41 File "/usr/lib/python3.4/socket.py", line 374, in readinto
17:47:41 return self._sock.recv_into(b)
17:47:41 File "/usr/lib/python3.4/ssl.py", line 769, in recv_into
17:47:41 return self.read(nbytes, buffer)
17:47:41 File "/usr/lib/python3.4/ssl.py", line 641, in read
17:47:41 v = self._sslobj.read(len, buffer)
17:47:41 ConnectionResetError: [Errno 104] Connection reset by peer
--
To view, visit http://gerrit.cloudera.org:8080/15043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284
Gerrit-Change-Number: 15043
Gerrit-PatchSet: 11
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Thu, 09 Apr 2020 01:22:04 +
Gerrit-HasComments: No
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Andrew Wong has uploaded a new patch set (#10) to the change originally created by Attila Bukor. ( http://gerrit.cloudera.org:8080/15681 ) Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. KUDU-3078 Add Ranger tests to master_authz-itest master_sentry-itest is renamed to master_authz-itest to generalize it and the tests were changed to also run with Ranger. A later patch will do the same for ts_sentry-itest. Ranger doesn't support adding new policy items (users and privileges) to existing policies, so MiniRanger::AddPolicy stores policies in a member variable and recreates them completely when a new item is added to an existing policy (resource). Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf --- M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/master_authz-itest.cc M src/kudu/master/ranger_authz_provider.cc M src/kudu/ranger/mini_ranger-test.cc M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client.cc M src/kudu/ranger/ranger_client.h 8 files changed, 738 insertions(+), 461 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15681/10 -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Bankim Bhavsar has uploaded a new patch set (#11) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. encoding-test: Clean up bitshuffle tests a little Also adds functions in random number generator to generate 128-bit integers. Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 --- M src/kudu/cfile/encoding-test.cc M src/kudu/util/random.h M src/kudu/util/random_util-test.cc M src/kudu/util/random_util.h 4 files changed, 159 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/43/15043/11 -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 11 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] net: make Sockaddr more generic for other address families
Volodymyr Verovkin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15690 ) Change subject: net: make Sockaddr more generic for other address families .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifff57e13a0e9b86b191d776ead67371b1e2ed4e3 Gerrit-Change-Number: 15690 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Thu, 09 Apr 2020 00:19:36 + Gerrit-HasComments: No
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. Patch Set 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/15043/10/src/kudu/util/random_util-test.cc File src/kudu/util/random_util-test.cc: http://gerrit.cloudera.org:8080/#/c/15043/10/src/kudu/util/random_util-test.cc@123 PS10, Line 123: static constexpr IntType min_val = std::numeric_limits::min() / 2; : static constexpr IntType max_val = std::numeric_limits::max() / 2; > Could you also exercise the min/max cases? Just to exercise what you mentio Done http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h File src/kudu/util/random_util.h: http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h@128 PS9, Line 128: > IMO it's best practice -- sentences without Oxford commas read ambiguously. Done http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h@146 PS9, Line 146: exception > Ah I see. Could you move this comment up to the be a function header so the I think comment is better placed close to the line that raises ASAN/UBSAN warning instead of the top of the function. -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 10 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Apr 2020 00:16:02 + Gerrit-HasComments: Yes
[kudu-CR] WIP: Add basic support for unix sockets
Todd Lipcon has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15691 Change subject: WIP: Add basic support for unix sockets .. WIP: Add basic support for unix sockets so far can't seem to reproduce any real speedup Change-Id: I40ce3e4e1b98f806f0c29d2fbd88789657218c4b --- M src/kudu/hms/hms_client-test.cc M src/kudu/rpc/acceptor_pool.cc M src/kudu/rpc/messenger.cc M src/kudu/rpc/reactor.cc M src/kudu/rpc/reactor.h M src/kudu/rpc/rpc-test-base.h M src/kudu/rpc/rpc-test.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/security/tls_socket.cc M src/kudu/security/tls_socket.h M src/kudu/util/net/net_util-test.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/sockaddr.cc M src/kudu/util/net/sockaddr.h M src/kudu/util/net/socket-test.cc M src/kudu/util/net/socket.cc M src/kudu/util/net/socket.h 17 files changed, 318 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/15691/1 -- To view, visit http://gerrit.cloudera.org:8080/15691 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I40ce3e4e1b98f806f0c29d2fbd88789657218c4b Gerrit-Change-Number: 15691 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Andrew Wong has uploaded a new patch set (#9) to the change originally created by Attila Bukor. ( http://gerrit.cloudera.org:8080/15681 ) Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. KUDU-3078 Add Ranger tests to master_authz-itest DONT_BUILD still haven't been able to run the tests successfully master_sentry-itest is renamed to master_authz-itest to generalize it and the tests were changed to also run with Ranger. Ranger doesn't support adding new policy items (users and privileges) to existing policies, so MiniRanger::AddPolicy stores policies in a member variable and recreates them completely when a new item is added to an existing policy (resource). Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf --- M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/master_authz-itest.cc M src/kudu/ranger/mini_ranger-test.cc M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h 5 files changed, 668 insertions(+), 434 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15681/9 -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 9 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] net: make Sockaddr more generic for other address families
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15690 ) Change subject: net: make Sockaddr more generic for other address families .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15690/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15690/2//COMMIT_MSG@10 PS2, Line 10: Unix domain sockets Curious about the motivation of this change. Plan to use Unix domain sockets instead of network for IPC on same host for better performance? -- To view, visit http://gerrit.cloudera.org:8080/15690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifff57e13a0e9b86b191d776ead67371b1e2ed4e3 Gerrit-Change-Number: 15690 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 08 Apr 2020 23:41:54 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.12.x) WIP [docs] Kudu 1.12 release notes draft
Alexey Serbin has uploaded a new patch set (#4) to the change originally created by Hao Hao. ( http://gerrit.cloudera.org:8080/15685 ) Change subject: WIP [docs] Kudu 1.12 release notes draft .. WIP [docs] Kudu 1.12 release notes draft WIP: * this is published just as working draft to iterate upon Change-Id: I300fb597a4eed36199ebf8760084e6df1fb04e9a --- M docs/release_notes.adoc 1 file changed, 104 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/15685/4 -- To view, visit http://gerrit.cloudera.org:8080/15685 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: I300fb597a4eed36199ebf8760084e6df1fb04e9a Gerrit-Change-Number: 15685 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin
[kudu-CR] net: make Sockaddr more generic for other address families
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15690 ) Change subject: net: make Sockaddr more generic for other address families .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.h File src/kudu/util/net/sockaddr.h: http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.h@39 PS2, Line 39: // Create an uninitialized socket. This instance must be assigned to before usage. : Sockaddr(); Can the default constructor be deleted instead? http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.h@69 PS2, Line 69: Parse a string IP address >From the implementation this function only parses IPV4 addresses. Comment >should be updated. http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.h@96 PS2, Line 96: return storage_.generic.ss_family; Should there be a check for is_initialized()? http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.cc File src/kudu/util/net/sockaddr.cc: http://gerrit.cloudera.org:8080/#/c/15690/2/src/kudu/util/net/sockaddr.cc@62 PS2, Line 62: struct sockaddr_in addr; : memset(, 0, sizeof(addr)); : addr.sin_family = AF_INET; : addr.sin_addr.s_addr = INADDR_ANY; : return Sockaddr(addr); Alternatively a static const object could be created just once and a copy simply returned. -- To view, visit http://gerrit.cloudera.org:8080/15690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifff57e13a0e9b86b191d776ead67371b1e2ed4e3 Gerrit-Change-Number: 15690 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 08 Apr 2020 23:32:42 + Gerrit-HasComments: Yes
[kudu-CR] cfile: clean up encoding-test to use fewer templates
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15044 ) Change subject: cfile: clean up encoding-test to use fewer templates .. Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15044/3/src/kudu/cfile/type_encodings.h File src/kudu/cfile/type_encodings.h: PS3: > I took a peek at it, changing the signature of functions in this interface Ack -- To view, visit http://gerrit.cloudera.org:8080/15044 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iba4464c2ea41107df96c68ea61576a0ea269277a Gerrit-Change-Number: 15044 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Apr 2020 23:15:38 + Gerrit-HasComments: Yes
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random.h File src/kudu/util/random.h: http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random.h@247 PS9, Line 247: return sizeof(IntType) == 4 ? rand->Next32() : : sizeof(IntType) == 8 ? rand->Next64() : rand->Next128(); > This is a constexpr function so can't use if/switch in C++11. See comment o Wow, C++14+ can't come soon enough. http://gerrit.cloudera.org:8080/#/c/15043/10/src/kudu/util/random_util-test.cc File src/kudu/util/random_util-test.cc: http://gerrit.cloudera.org:8080/#/c/15043/10/src/kudu/util/random_util-test.cc@123 PS10, Line 123: static constexpr IntType min_val = std::numeric_limits::min() / 2; : static constexpr IntType max_val = std::numeric_limits::max() / 2; Could you also exercise the min/max cases? Just to exercise what you mentioned in the random_util comment. http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h File src/kudu/util/random_util.h: http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h@128 PS9, Line 128: > Oxford comma is optional unless our coding guidelines explicitly require it IMO it's best practice -- sentences without Oxford commas read ambiguously. I suspect you'll find more sentences with it than without it in this codebase. But if you feel strongly against it, I'm fine leaving it as is. http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h@146 PS9, Line 146: exception > This is the comment for ASAN exception added for this function. See L130. Ah I see. Could you move this comment up to the be a function header so the explanation is closer to the more interesting code? -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 10 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Apr 2020 23:13:42 + Gerrit-HasComments: Yes
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15042 ) Change subject: cfile: change BlockBuilder API to yield a vector of Slices .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Gerrit-Change-Number: 15042 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Apr 2020 23:03:14 + Gerrit-HasComments: No
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15681 ) Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/15681/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15681/8//COMMIT_MSG@9 PS8, Line 9: master_sentry-itest is renamed to master_authz-itest to generalize it There will be a follow up patch to add ranger tests for ts integration test (such as ts_ranger-itest.cc)? -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 08 Apr 2020 22:35:18 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/15681 )
Change subject: KUDU-3078 Add Ranger tests to master_authz-itest
..
Patch Set 7:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/15681/7/src/kudu/integration-tests/master_authz-itest.cc
File src/kudu/integration-tests/master_authz-itest.cc:
http://gerrit.cloudera.org:8080/#/c/15681/7/src/kudu/integration-tests/master_authz-itest.cc@443
PS7, Line 443: const uni
nit: indent.
http://gerrit.cloudera.org:8080/#/c/15681/7/src/kudu/integration-tests/master_authz-itest.cc@477
PS7, Line 477: policy.tables.emplace_back("*");
This can be removed as only Database level privilege is required.
http://gerrit.cloudera.org:8080/#/c/15681/7/src/kudu/integration-tests/master_authz-itest.cc@491
PS7, Line 491: SleepFor(MonoDelta::FromMilliseconds(1500))
If the sleep is for waiting the polling interval to finish in Ranger client, we
can even make it smaller instead of sleep here?
http://gerrit.cloudera.org:8080/#/c/15681/7/src/kudu/integration-tests/master_authz-itest.cc@510
PS7, Line 510: policy_new_table.tables.emplace_back("*")
This can be removed.
http://gerrit.cloudera.org:8080/#/c/15681/7/src/kudu/integration-tests/master_authz-itest.cc@540
PS7, Line 540: ParseHiveTableIdentifier
We should use ParseRangerTableIdentifier?
http://gerrit.cloudera.org:8080/#/c/15681/7/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/15681/7/src/kudu/master/ranger_authz_provider.cc@199
PS7, Line 199: CHECK_OK(client_.AuthorizeActionMultipleColumns(user,
ActionPB::SELECT, table_name,
:
_names));
But the subprocess execution can also return a non OK() status?
--
To view, visit http://gerrit.cloudera.org:8080/15681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf
Gerrit-Change-Number: 15681
Gerrit-PatchSet: 7
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 08 Apr 2020 22:30:10 +
Gerrit-HasComments: Yes
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Bankim Bhavsar has uploaded a new patch set (#7) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/15042 ) Change subject: cfile: change BlockBuilder API to yield a vector of Slices .. cfile: change BlockBuilder API to yield a vector of Slices When blocks are appended to cfiles at the IO layer, we already have the ability to write multiple slices using a vectored IO. Prior to this patch, the BlockBuilder API was restricted to returning a single slice, whereas it would be more convenient in some cases to be able to return multiple slices (eg separating the header from the data). This new functionality is used by BinaryDictBlockBuilder to avoid an extra copy in Finish(). Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db --- M src/kudu/cfile/binary_dict_block.cc M src/kudu/cfile/binary_dict_block.h M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/binary_plain_block.h M src/kudu/cfile/binary_prefix_block.cc M src/kudu/cfile/binary_prefix_block.h M src/kudu/cfile/block_encodings.h M src/kudu/cfile/bshuf_block.cc M src/kudu/cfile/bshuf_block.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/encoding-test.cc M src/kudu/cfile/plain_bitmap_block.h M src/kudu/cfile/plain_block.h M src/kudu/cfile/rle_block.h 14 files changed, 115 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/15042/7 -- To view, visit http://gerrit.cloudera.org:8080/15042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Gerrit-Change-Number: 15042 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] cfile: clean up encoding-test to use fewer templates
Bankim Bhavsar has uploaded a new patch set (#4) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/15044 ) Change subject: cfile: clean up encoding-test to use fewer templates .. cfile: clean up encoding-test to use fewer templates This test made heavy use of templates, which made things overly complicated and hard to follow. All of the block builders/decoders already implement common interfaces, so we can use runtime polymorphism instead for the majority of code here. Change-Id: Iba4464c2ea41107df96c68ea61576a0ea269277a --- M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/encoding-test.cc M src/kudu/cfile/type_encodings.cc M src/kudu/cfile/type_encodings.h 5 files changed, 270 insertions(+), 320 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/15044/4 -- To view, visit http://gerrit.cloudera.org:8080/15044 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iba4464c2ea41107df96c68ea61576a0ea269277a Gerrit-Change-Number: 15044 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Bankim Bhavsar has uploaded a new patch set (#10) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. encoding-test: Clean up bitshuffle tests a little Also adds functions in random number generator to generate 128-bit integers. Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 --- M src/kudu/cfile/encoding-test.cc M src/kudu/util/random.h M src/kudu/util/random_util-test.cc M src/kudu/util/random_util.h 4 files changed, 151 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/43/15043/10 -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 10 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. Patch Set 9: (7 comments) http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random.h File src/kudu/util/random.h: http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random.h@91 PS9, Line 91: upto > nit: missing space Done http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random.h@236 PS9, Line 236: Next128 > nit: for consistency, add () Done http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random.h@247 PS9, Line 247: return sizeof(IntType) == 4 ? rand->Next32() : : sizeof(IntType) == 8 ? rand->Next64() : rand->Next128(); > nit: may be simpler with a switch. Same below This is a constexpr function so can't use if/switch in C++11. See comment on line 242. Moved the comment right above this line for better readability. http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random.h@252 PS9, Line 252: integer > nit: maybe specify that this can be signed or unsigned Done http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h File src/kudu/util/random_util.h: http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h@128 PS9, Line 128: > nit: add a comma; same elsewhere. Oxford comma is optional unless our coding guidelines explicitly require it. So I'm going to drop this comment. http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h@131 PS9, Line 131: std::vector CreateRandomIntegersInRange(const int num_values, > Could you also add a small test for this in util/random_util-test.cc? Done http://gerrit.cloudera.org:8080/#/c/15043/9/src/kudu/util/random_util.h@146 PS9, Line 146: exception > What exception is this? This is the comment for ASAN exception added for this function. See L130. -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 9 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Apr 2020 22:11:37 + Gerrit-HasComments: Yes
[kudu-CR] cfile: clean up encoding-test to use fewer templates
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15044 ) Change subject: cfile: clean up encoding-test to use fewer templates .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/15044/3/src/kudu/cfile/type_encodings.h File src/kudu/cfile/type_encodings.h: PS3: > Oops, yea. I took a peek at it, changing the signature of functions in this interface would entail changing function pointers and what appears bunch of templatized specialization for encodings. It's best done separately as a follow-up change. http://gerrit.cloudera.org:8080/#/c/15044/3/src/kudu/cfile/type_encodings.h@53 PS3, Line 53: // iter parameter will only be used when it is dictionary encoding > nit: not your fault, but could you update this to mention that this is an i Done -- To view, visit http://gerrit.cloudera.org:8080/15044 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iba4464c2ea41107df96c68ea61576a0ea269277a Gerrit-Change-Number: 15044 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Apr 2020 22:11:28 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15681 to look at the new patch set (#8). Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. KUDU-3078 Add Ranger tests to master_authz-itest master_sentry-itest is renamed to master_authz-itest to generalize it and the tests were changed to also run with Ranger. Ranger doesn't support adding new policy items (users and privileges) to existing policies, so MiniRanger::AddPolicy stores policies in a member variable and recreates them completely when a new item is added to an existing policy (resource). This broke an existing test in mini_ranger-test which was testing persistence by expecting an error after adding an existing policy after restarting Ranger, this test is fetching the number of policies now instead. These new integration tests uncovered a previously unknown bug in Ranger authorization provider and Ranger client which is also fixed now. Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf --- M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/master_authz-itest.cc M src/kudu/master/ranger_authz_provider.cc M src/kudu/ranger/mini_ranger-test.cc M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/ranger/ranger_client.h 9 files changed, 637 insertions(+), 244 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15681/8 -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] net: make Sockaddr more generic for other address families
Hello Mike Percy, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15690 to look at the new patch set (#2). Change subject: net: make Sockaddr more generic for other address families .. net: make Sockaddr more generic for other address families This paves the way for supporting other socket address types: namely IPv6 and Unix domain sockets. This patch doesn't add any new support, but increases the size of Sockaddr appropriately and making the methods more generic. The approach here is to use a union inside of the Sockaddr class which could store an address of any type. An alternative (taken by a long-ago attempt at unix sockets) would be to make Sockaddr an abstract base class with derived classes for each socket type. I went with this approach since all of the POSIX socket APIs expect this "tagged union" approach -- the same APIs are used whether we want to talk to a unix socket or an IPv4. The downside of this approach is a slightly larger Sockaddr object footprint, but we shouldn't have a lot of these on the heap. One functional change: the default constructor for Sockaddr now creates an "uninitialized" instance, whereas it used to create a wildcard address. That's now an explicit static method. Change-Id: Ifff57e13a0e9b86b191d776ead67371b1e2ed4e3 --- M src/kudu/clock/builtin_ntp.cc M src/kudu/rpc/mt-rpc-test.cc M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/proxy.cc M src/kudu/rpc/rpc-test-base.h M src/kudu/rpc/rpc-test.cc M src/kudu/server/webserver.cc M src/kudu/tserver/scanners.cc M src/kudu/util/net/net_util-test.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/sockaddr.cc M src/kudu/util/net/sockaddr.h M src/kudu/util/net/socket.cc 13 files changed, 188 insertions(+), 81 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/15690/2 -- To view, visit http://gerrit.cloudera.org:8080/15690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifff57e13a0e9b86b191d776ead67371b1e2ed4e3 Gerrit-Change-Number: 15690 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] net: make Sockaddr more generic for other address families
Hello Mike Percy, Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15690 to review the following change. Change subject: net: make Sockaddr more generic for other address families .. net: make Sockaddr more generic for other address families This paves the way for supporting other socket address types: namely IPv6 and Unix domain sockets. This patch doesn't add any new support, but increases the size of Sockaddr appropriately and making the methods more generic. The approach here is to use a union inside of the Sockaddr class which could store an address of any type. An alternative (taken by a long-ago attempt at unix sockets) would be to make Sockaddr an abstract base class with derived classes for each socket type. I went with this approach since all of the POSIX socket APIs expect this "tagged union" approach -- the same APIs are used whether we want to talk to a unix socket or an IPv4. The downside of this approach is a slightly larger Sockaddr object footprint, but we shouldn't have a lot of these on the heap. One functional change: the default constructor for Sockaddr now creates an "uninitialized" instance, whereas it used to create a wildcard address. That's now an explicit static method. Change-Id: Ifff57e13a0e9b86b191d776ead67371b1e2ed4e3 --- M src/kudu/clock/builtin_ntp.cc M src/kudu/rpc/mt-rpc-test.cc M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/proxy.cc M src/kudu/rpc/rpc-test-base.h M src/kudu/rpc/rpc-test.cc M src/kudu/server/webserver.cc M src/kudu/tserver/scanners.cc M src/kudu/util/net/net_util-test.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/sockaddr.cc M src/kudu/util/net/sockaddr.h M src/kudu/util/net/socket.cc 13 files changed, 179 insertions(+), 81 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/15690/1 -- To view, visit http://gerrit.cloudera.org:8080/15690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifff57e13a0e9b86b191d776ead67371b1e2ed4e3 Gerrit-Change-Number: 15690 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-3078 Refactor Sentry integration tests
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/15671 ) Change subject: KUDU-3078 Refactor Sentry integration tests .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/15671/7/src/kudu/integration-tests/ts_sentry-itest.cc File src/kudu/integration-tests/ts_sentry-itest.cc: http://gerrit.cloudera.org:8080/#/c/15671/7/src/kudu/integration-tests/ts_sentry-itest.cc@362 PS7, Line 362: StartClusterWithOpts(opts); > warning: std::move of the const variable 'opts' has no effect; remove std:: Done -- To view, visit http://gerrit.cloudera.org:8080/15671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieff25eaf7a41cf7d8fe965738907e67bbd9b9051 Gerrit-Change-Number: 15671 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 08 Apr 2020 21:49:15 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15681 to look at the new patch set (#7). Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. KUDU-3078 Add Ranger tests to master_authz-itest master_sentry-itest is renamed to master_authz-itest to generalize it and the tests were changed to also run with Ranger. Ranger doesn't support adding new policy items (users and privileges) to existing policies, so MiniRanger::AddPolicy stores policies in a member variable and recreates them completely when a new item is added to an existing policy (resource). This broke an existing test in mini_ranger-test which was testing persistence by expecting an error after adding an existing policy after restarting Ranger, this test is fetching the number of policies now instead. These new integration tests uncovered a previously unknown bug in Ranger authorization provider and Ranger client which is also fixed now. Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf --- M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/master_authz-itest.cc M src/kudu/master/ranger_authz_provider.cc M src/kudu/ranger/mini_ranger-test.cc M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/ranger/ranger_client.h 9 files changed, 635 insertions(+), 243 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15681/7 -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 7 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3078 Refactor Sentry integration tests
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15671 to look at the new patch set (#8). Change subject: KUDU-3078 Refactor Sentry integration tests .. KUDU-3078 Refactor Sentry integration tests This commit refactors the existing Sentry integration tests to prepare for Ranger integration tests. It changes HMS and Sentry integration test base classes to test harnesses that don't inherit from Gtest to simplify inheritance when using typed tests. Change-Id: Ieff25eaf7a41cf7d8fe965738907e67bbd9b9051 --- 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/integration-tests/master_sentry-itest.cc M src/kudu/integration-tests/ts_sentry-itest.cc 5 files changed, 500 insertions(+), 215 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/15671/8 -- To view, visit http://gerrit.cloudera.org:8080/15671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieff25eaf7a41cf7d8fe965738907e67bbd9b9051 Gerrit-Change-Number: 15671 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15681 to look at the new patch set (#6). Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. KUDU-3078 Add Ranger tests to master_authz-itest master_sentry-itest is renamed to master_authz-itest to generalize it and the tests were changed to also run with Ranger. Ranger doesn't support adding new policy items (users and privileges) to existing policies, so MiniRanger::AddPolicy stores policies in a member variable and recreates them completely when a new item is added to an existing policy (resource). These new integration tests uncovered a previously unknown bug in Ranger authorization provider and Ranger client which is also fixed now. Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf --- M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/master_authz-itest.cc M src/kudu/master/ranger_authz_provider.cc M src/kudu/ranger/mini_ranger-test.cc M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/ranger/ranger_client.h 9 files changed, 623 insertions(+), 238 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15681/6 -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 6 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15681 to look at the new patch set (#5). Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. KUDU-3078 Add Ranger tests to master_authz-itest master_sentry-itest is renamed to master_authz-itest to generalize it and the tests were changed to also run with Ranger. Ranger doesn't support adding new policy items (users and privileges) to existing policies, so MiniRanger::AddPolicy stores policies in a member variable and recreates them completely when a new item is added to an existing policy (resource). These new integration tests uncovered a previously unknown bug in Ranger authorization provider and Ranger client which is also fixed now. Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf --- M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/master_authz-itest.cc M src/kudu/master/ranger_authz_provider.cc M src/kudu/ranger/mini_ranger-test.cc M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/ranger/ranger_client.h 9 files changed, 624 insertions(+), 238 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15681/5 -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 5 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] columnar serialization: fix optimized GCC build
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15680 ) Change subject: columnar_serialization: fix optimized GCC build .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15680/2/src/kudu/common/columnar_serialization.cc File src/kudu/common/columnar_serialization.cc: http://gerrit.cloudera.org:8080/#/c/15680/2/src/kudu/common/columnar_serialization.cc@389 PS2, Line 389: long long int > Since this is breaking some builds I will merge and leave that comment and SGTM I was just curious whether it will break macOS build because of that. But it seems it's all right now, if I'm not mistaken. -- To view, visit http://gerrit.cloudera.org:8080/15680 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b69470b238dae7a33fdd6b44cb8be57a26501d7 Gerrit-Change-Number: 15680 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 08 Apr 2020 20:51:39 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.12.x) WIP [docs] Kudu 1.12 release notes draft
Alexey Serbin has uploaded a new patch set (#3) to the change originally created by Hao Hao. ( http://gerrit.cloudera.org:8080/15685 ) Change subject: WIP [docs] Kudu 1.12 release notes draft .. WIP [docs] Kudu 1.12 release notes draft WIP: * this is published just as working draft to iterate upon Change-Id: I300fb597a4eed36199ebf8760084e6df1fb04e9a --- M docs/release_notes.adoc 1 file changed, 77 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/15685/3 -- To view, visit http://gerrit.cloudera.org:8080/15685 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: I300fb597a4eed36199ebf8760084e6df1fb04e9a Gerrit-Change-Number: 15685 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin
[kudu-CR](branch-1.12.x) WIP [docs] Kudu 1.12 release notes draft
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15685 ) Change subject: WIP [docs] Kudu 1.12 release notes draft .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@58 PS2, Line 58: Kudu now offers a tool to quiesce tablet servers > "The `kudu tserver quiesce` tool is added to quiesce tablet servers." Done http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@63 PS2, Line 63: TODO: automatic tablet rebalancing > Kudu can automatically rebalance tablet replicas among tablet servers. The Thank you! I added this with minor modifications. http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@74 PS2, Line 74: kudu fs update_dirs > wrap in backticks Done http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@75 PS2, Line 75: recovery > "recover" Done http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@81 PS2, Line 81: maintenance > "maintenance operation" Done http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@84 PS2, Line 84: certain workloads > "workloads that contained updates" Done -- To view, visit http://gerrit.cloudera.org:8080/15685 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: comment Gerrit-Change-Id: I300fb597a4eed36199ebf8760084e6df1fb04e9a Gerrit-Change-Number: 15685 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Wed, 08 Apr 2020 20:40:45 + Gerrit-HasComments: Yes
[kudu-CR] WIP: KUDU-3097 whether master keep record could be configurable
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15616 ) Change subject: WIP: KUDU-3097 whether master keep record could be configurable .. Patch Set 4: > But it would cause some inconsistent between table and tablets. Cause tablet > should be keep in memory forever in order to not overlap with previously > deleted table. > E.g table t1 with tablet uuid 1,2,3 > Then I deleted it, the tablet uuid 1,2,3 shouldn't be bind to other new table > t2,t3 ... > And also I have to handle other things like this, table not exists but tablet > still in tablet_map_. > > So if I want to implement this behave, I should handle things like this. > > Is my understand about this right? Table and tablet UUIDs are expected to be universally unique, so you don't need to preserve them in-memory just to avoid reusing them in the future. There shouldn't ever be any collisions. Furthermore, table and tablet metadata should always be consistent with one another: either both should be preserved, or both should be deleted. It'd be problematic to preserve tablet metadata while discarding table metadata. Hope this helps! -- To view, visit http://gerrit.cloudera.org:8080/15616 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1088080d706edadeed9d93f54704c7f4bfe2b6a2 Gerrit-Change-Number: 15616 Gerrit-PatchSet: 4 Gerrit-Owner: wangning <1994wangn...@gmail.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: wangning <1994wangn...@gmail.com> Gerrit-Comment-Date: Wed, 08 Apr 2020 20:29:42 + Gerrit-HasComments: No
[kudu-CR](branch-1.12.x) WIP [docs] Kudu 1.12 release notes draft
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/15685 ) Change subject: WIP [docs] Kudu 1.12 release notes draft .. Patch Set 2: (1 comment) Drafted a release note for auto-rebalancing! http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@63 PS2, Line 63: TODO: automatic tablet rebalancing Kudu can automatically rebalance tablet replicas among tablet servers. The background task can be enabled by setting the `auto_rebalancing_enabled` flag in the catalog manager. Before starting auto-rebalancing on an existing cluster, the CLI rebalancer tool should be run first (see link:https://issues.apache.org/jira/browse/KUDU-2780[KUDU-2780]). -- To view, visit http://gerrit.cloudera.org:8080/15685 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: comment Gerrit-Change-Id: I300fb597a4eed36199ebf8760084e6df1fb04e9a Gerrit-Change-Number: 15685 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Wed, 08 Apr 2020 20:06:28 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3078 Refactor Sentry integration tests
Attila Bukor has posted comments on this change. (
http://gerrit.cloudera.org:8080/15671 )
Change subject: KUDU-3078 Refactor Sentry integration tests
..
Patch Set 7:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/master_hms-itest.cc@107
PS6, Line 107: return HmsITestHarness::CreateKuduTable(database_name,
table_name, client_, timeout);
> warning: static member accessed through instance [readability-static-access
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/master_sentry-itest.cc
File src/kudu/integration-tests/master_sentry-itest.cc:
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/master_sentry-itest.cc@211
PS6, Line 211: static Status CreateTable(const OperationParams& p,
> warning: method 'CreateTable' can be made static [readability-convert-membe
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/master_sentry-itest.cc@257
PS6, Line 257: static Status GetTableLocations(const OperationParams& p,
> warning: method 'GetTableLocations' can be made static [readability-convert
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc
File src/kudu/integration-tests/ts_sentry-itest.cc:
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@249
PS6, Line 249: constexpr int kNumUsers = 3;
> warning: 'kNumUsers' is a static definition in anonymous namespace; static
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@250
PS6, Line 250: constexpr const char* kAdminGroup = "admin";
> warning: 'kAdminGroup' is a static definition in anonymous namespace; stati
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@252
PS6, Line 252: constexpr int kNumTables = 3;
> warning: 'kNumTables' is a static definition in anonymous namespace; static
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@253
PS6, Line 253: constexpr int kNumColsPerTable = 3;
> warning: 'kNumColsPerTable' is a static definition in anonymous namespace;
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@254
PS6, Line 254: constexpr const char* kDb = "db";
> warning: 'kDb' is a static definition in anonymous namespace; static is red
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@255
PS6, Line 255: constexpr const char* kTablePrefix = "table";
> warning: 'kTablePrefix' is a static definition in anonymous namespace; stat
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@256
PS6, Line 256: constexpr const char* kAdminRole = "kudu-admin";
> warning: 'kAdminRole' is a static definition in anonymous namespace; static
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@258
PS6, Line 258: constexpr int kAuthzTokenTTLSecs = 1;
> warning: 'kAuthzTokenTTLSecs' is a static definition in anonymous namespace
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@259
PS6, Line 259: constexpr int kAuthzCacheTTLMultiplier = 3;
> warning: 'kAuthzCacheTTLMultiplier' is a static definition in anonymous nam
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@269
PS6, Line 269: ExternalMiniClusterOptions)>& cluster_start_func) {
> warning: the parameter 'cluster_start_func' is copied for each invocation b
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@325
PS6, Line 325: Status CreateTable(const string& table_ident, const
shared_ptr& client) {
> warning: the const qualified parameter 'client' is copied for each invocati
Done
http://gerrit.cloudera.org:8080/#/c/15671/6/src/kudu/integration-tests/ts_sentry-itest.cc@360
PS6, Line 360: harness_.SetUp([&] (const ExternalMiniClusterOptions& opts)
->
> warning: the parameter 'opts' is copied for each invocation but only used a
Done
--
To view, visit http://gerrit.cloudera.org:8080/15671
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieff25eaf7a41cf7d8fe965738907e67bbd9b9051
Gerrit-Change-Number: 15671
Gerrit-PatchSet: 7
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 08 Apr 2020 20:05:24 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3078 Refactor Sentry integration tests
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15671 to look at the new patch set (#7). Change subject: KUDU-3078 Refactor Sentry integration tests .. KUDU-3078 Refactor Sentry integration tests This commit refactors the existing Sentry integration tests to prepare for Ranger integration tests. It changes HMS and Sentry integration test base classes to test harnesses that don't inherit from Gtest to simplify inheritance when using typed tests. Change-Id: Ieff25eaf7a41cf7d8fe965738907e67bbd9b9051 --- 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/integration-tests/master_sentry-itest.cc M src/kudu/integration-tests/ts_sentry-itest.cc 5 files changed, 500 insertions(+), 215 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/15671/7 -- To view, visit http://gerrit.cloudera.org:8080/15671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieff25eaf7a41cf7d8fe965738907e67bbd9b9051 Gerrit-Change-Number: 15671 Gerrit-PatchSet: 7 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Attila Bukor has posted comments on this change. (
http://gerrit.cloudera.org:8080/15681 )
Change subject: KUDU-3078 Add Ranger tests to master_authz-itest
..
Patch Set 4:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/15681/1/src/kudu/integration-tests/master_authz-itest.cc
File src/kudu/integration-tests/master_authz-itest.cc:
http://gerrit.cloudera.org:8080/#/c/15681/1/src/kudu/integration-tests/master_authz-itest.cc@155
PS1, Line 155:
> nit: Is this needed? Doesn't only the SentryAuthzITestHarness need the HMS
Done
http://gerrit.cloudera.org:8080/#/c/15681/1/src/kudu/integration-tests/master_authz-itest.cc@300
PS1, Line 300:make_optional(kAdminUser),
cluster, client);
: CheckTable(kDatabaseName, kSecondTable,
:make_optional(kAdminUser),
cluster, client);
: return Status::OK();
> nit: maybe rename these to SetUpExternalMiniServiceOpts() and SetUpExternal
Done
http://gerrit.cloudera.org:8080/#/c/15681/1/src/kudu/integration-tests/master_authz-itest.cc@920
PS1, Line 920: // in Kudu and the
> If you replace this with ASSERT_STR_MATCHES, you can reuse it for ranger to
Hao said she already used the upper-case version in systest, so that's why I
decided against making them consistent. Thanks for the tip.
http://gerrit.cloudera.org:8080/#/c/15681/1/src/kudu/integration-tests/master_authz-itest.cc@1031
PS1, Line 1031: {
> Remove
Done
http://gerrit.cloudera.org:8080/#/c/15681/1/src/kudu/integration-tests/master_authz-itest.cc@1151
PS1, Line 1151:
> We may want to consider using testing::Combine on some kSentry/kRanger enum
How do you feel about adding a TODO to refactor it once we support C++14? With
C++14 we could templatize this.
http://gerrit.cloudera.org:8080/#/c/15681/3/src/kudu/integration-tests/master_authz-itest.cc
File src/kudu/integration-tests/master_authz-itest.cc:
http://gerrit.cloudera.org:8080/#/c/15681/3/src/kudu/integration-tests/master_authz-itest.cc@645
PS3, Line 645:
> I'm a bit surprised that this compiles. Maybe I'm missing something, but th
Done
http://gerrit.cloudera.org:8080/#/c/15681/3/src/kudu/integration-tests/master_authz-itest.cc@689
PS3, Line 689: Status StopAuthzProvider() {
: return harness_.StopAuthzProvider(cluster_);
: }
> Can we not refactor a bit to have the first argument be a MasterAuthzITestH
I think this could be done, but that would require us to rewrite the methods in
the harness to expect a pointer to the cluster as an argument instead of the
client and create their own client which feels weird. How about rewriting this
part as well when we can templatize variables? (C++14)
http://gerrit.cloudera.org:8080/#/c/15681/3/src/kudu/ranger/mini_ranger.h
File src/kudu/ranger/mini_ranger.h:
http://gerrit.cloudera.org:8080/#/c/15681/3/src/kudu/ranger/mini_ranger.h@54
PS3, Line 54: // Policy key used for searching policies_
> nit: could you mention what the fields are?
Done
http://gerrit.cloudera.org:8080/#/c/15681/3/src/kudu/ranger/mini_ranger.h@59
PS3, Line 59: // The AuthorizationPolicy contains a set of privileges on a
resource to one or
: // more users. 'items' is a vector of user-list of actions pair.
This struct can
: // be used to create new Ranger policies in tests.
> nit: should mention the policy name will be based on its contents.
Done
http://gerrit.cloudera.org:8080/#/c/15681/3/src/kudu/ranger/mini_ranger.h@191
PS3, Line 191: cies since s
> nit: "resources"?
Done
http://gerrit.cloudera.org:8080/#/c/15681/3/src/kudu/ranger/mini_ranger.h@192
PS3, Line 192: used fo
> nit: Ranger
Done
--
To view, visit http://gerrit.cloudera.org:8080/15681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf
Gerrit-Change-Number: 15681
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 08 Apr 2020 20:05:28 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3078 Add Ranger tests to master authz-itest
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15681 to look at the new patch set (#4). Change subject: KUDU-3078 Add Ranger tests to master_authz-itest .. KUDU-3078 Add Ranger tests to master_authz-itest master_sentry-itest is renamed to master_authz-itest to generalize it and the tests were changed to also run with Ranger. Ranger doesn't support adding new policy items (users and privileges) to existing policies, so MiniRanger::AddPolicy stores policies in a member variable and recreates them completely when a new item is added to an existing policy (resource). Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf --- M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/master_authz-itest.cc M src/kudu/ranger/mini_ranger-test.cc M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h 5 files changed, 612 insertions(+), 217 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15681/4 -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] tserver: add locking around an RPC's usage of a Scanner
Hello Tidy Bot, Andrew Wong, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15677 to look at the new patch set (#2). Change subject: tserver: add locking around an RPC's usage of a Scanner .. tserver: add locking around an RPC's usage of a Scanner Prior to this patch, there was little to prevent multiple RPCs from trying to access and work with a single Scanner object other than the call sequence ID checks. In practice, those checks were fairly likely to prevent races -- there was only a very small window of time in between looking up a scanner and checking its sequence ID -- but it was buggy nonetheless. In addition, the lack of reasonable locking meant that some variables like scanner::num_rows_returned_ were needlessly atomic. In certain workloads this actually took a measurable amount of CPU. This patch adds a new test that would reliably crash the tserver and trigger TSAN warnings prior to the fix. It adds some explicit locking around the Scanner object and asserts that most methods on the scanner require the lock to be held. I benchmarked this by running perf-stat on a tserver and 'kudu perf table_scan' to count the rows in a table 200 times (ie scan with no columns projected). This is a best case for seeing the value of the optimization. Before: Performance counter stats for './build/latest/bin/kudu tserver run -fs-wal-dir /tmp/ts': 11,753.59 msec task-clock#1.166 CPUs utilized 7,174 context-switches #0.610 K/sec 807 cpu-migrations#0.069 K/sec 14,909 page-faults #0.001 M/sec 32,447,480,644 cycles#2.761 GHz 49,275,463,523 instructions #1.52 insn per cycle 7,570,941,653 branches # 644.139 M/sec 14,271,190 branch-misses #0.19% of all branches 10.082020598 seconds time elapsed 11.561886000 seconds user 0.340585000 seconds sys After: Performance counter stats for './build/latest/bin/kudu.opt tserver run -fs-wal-dir /tmp/ts': 9,426.45 msec task-clock#1.010 CPUs utilized 6,906 context-switches #0.733 K/sec 892 cpu-migrations#0.095 K/sec 15,078 page-faults #0.002 M/sec 26,127,343,920 cycles#2.772 GHz 48,101,748,066 instructions #1.84 insn per cycle 7,402,811,470 branches # 785.323 M/sec 13,857,599 branch-misses #0.19% of all branches 9.335786317 seconds time elapsed 9.258446000 seconds user 0.315547000 seconds sys (1.24x fewer cycles for the same work) When I actually scan one column of data, the improvement is only about 1% since other CPU consumption is dominant. Change-Id: I3591e85a07aefadaf7fb05768109c8a261a8828e --- M src/kudu/tserver/scanners-test.cc M src/kudu/tserver/scanners.cc M src/kudu/tserver/scanners.h M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc 5 files changed, 271 insertions(+), 127 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/15677/2 -- To view, visit http://gerrit.cloudera.org:8080/15677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3591e85a07aefadaf7fb05768109c8a261a8828e Gerrit-Change-Number: 15677 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR](branch-1.12.x) WIP [docs] Kudu 1.12 release notes draft
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15685 ) Change subject: WIP [docs] Kudu 1.12 release notes draft .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@58 PS2, Line 58: Kudu now offers a tool to quiesce tablet servers "The `kudu tserver quiesce` tool is added to quiesce tablet servers." http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@74 PS2, Line 74: kudu fs update_dirs wrap in backticks http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@75 PS2, Line 75: recovery "recover" http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@81 PS2, Line 81: maintenance "maintenance operation" http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@84 PS2, Line 84: certain workloads "workloads that contained updates" -- To view, visit http://gerrit.cloudera.org:8080/15685 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: comment Gerrit-Change-Id: I300fb597a4eed36199ebf8760084e6df1fb04e9a Gerrit-Change-Number: 15685 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Wed, 08 Apr 2020 18:51:29 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3007. Support building Kudu on aarch64 platform
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, huangtianhua...@gmail.com, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14964 to look at the new patch set (#15). Change subject: KUDU-3007. Support building Kudu on aarch64 platform .. KUDU-3007. Support building Kudu on aarch64 platform This change try to modify and make Kudu can build sucessfully on both x86_64 and aarch64 platform. Change-Id: I2953519c5d28de17e6b2bb7094abab0c1cd12c97 --- M CMakeLists.txt M README.adoc M build-support/jenkins/build-and-test.sh M java/kudu-client/build.gradle M java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/bitshuffle_arch_wrapper.cc M src/kudu/codegen/codegen-test.cc M src/kudu/common/columnar_serialization.cc M src/kudu/common/key_encoder.h M src/kudu/common/zp7.cc A src/kudu/gutil/atomicops-internals-arm64.h M src/kudu/gutil/atomicops-internals-x86.cc M src/kudu/gutil/atomicops.h M src/kudu/gutil/cpu.cc M src/kudu/gutil/cycleclock-inl.h M src/kudu/gutil/dynamic_annotations.h M src/kudu/gutil/port.h M src/kudu/gutil/spinlock.h M src/kudu/gutil/spinlock_linux-inl.h M src/kudu/util/block_bloom_filter.cc M src/kudu/util/char_util.cc M src/kudu/util/debug-util.cc M src/kudu/util/debug-util.h M src/kudu/util/group_varint-inl.h M src/kudu/util/group_varint-test.cc M src/kudu/util/init.cc M src/kudu/util/memory/memory.cc M src/kudu/util/notification.h A src/kudu/util/sse2neon.h M src/kudu/util/striped64.cc M thirdparty/build-definitions.sh 32 files changed, 3,798 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/14964/15 -- To view, visit http://gerrit.cloudera.org:8080/14964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2953519c5d28de17e6b2bb7094abab0c1cd12c97 Gerrit-Change-Number: 14964 Gerrit-PatchSet: 15 Gerrit-Owner: liusheng Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: liusheng
