[kudu-CR] KUDU-3475: Update to the most recent sse2neon.h
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19847 ) Change subject: KUDU-3475: Update to the most recent sse2neon.h .. KUDU-3475: Update to the most recent sse2neon.h ARM builds on Ubuntu 20.04 hit an error about the redeclaration of vld1q_u8_x4 in sse2neon.h. This is because Ubuntu 20.04 uses GCC 9.4, which has its own definition of vld1q_u8_x4, and the preprocessor checks in the current sse2neon.h don't account for this correctly. Newer versions of sse2neon.h have namespaced their macros so that vld1q_u8_x4 is no longer used and improved the checks for GCC versions. This updates sse2neon.h to the latest available (commithash b7417bc). sse2neon.h has a large number of lint issues related to style (whitespace/braces, readability/casting) and none look like real correctness issues, so this exempts sse2neon.h from lint. Testing: - Built Kudu on Ubuntu 20.04 ARM64 and Ubuntu 18.04 ARM64 Change-Id: Id156a0b2c3984f9cb1032fa7747d7f9ed76c1f8c Reviewed-on: http://gerrit.cloudera.org:8080/19847 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Yingchun Lai --- M build-support/lint.sh M src/kudu/util/sse2neon.h 2 files changed, 8,813 insertions(+), 3,028 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved Yingchun Lai: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/19847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id156a0b2c3984f9cb1032fa7747d7f9ed76c1f8c Gerrit-Change-Number: 19847 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai
[kudu-CR] KUDU-3474: Add zlib as dependency of libunwind for ARM
Alexey Serbin has removed a vote on this change. Change subject: KUDU-3474: Add zlib as dependency of libunwind for ARM .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/19846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Iee87b908a5771c2bd7c1a653504a1449d0792280 Gerrit-Change-Number: 19846 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3474: Add zlib as dependency of libunwind for ARM
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19846 ) Change subject: KUDU-3474: Add zlib as dependency of libunwind for ARM .. KUDU-3474: Add zlib as dependency of libunwind for ARM On ARM, libunwind includes support for compressed .debug_info sections if zlib is available during compilation. This means that libunwind can require zlib at link time, but only for ARM architecture. This modifies the build to add a zlib dependency for libunwind if the architecture is ARM. It also reorders the thirdparty build so that zlib is built before libunwind. This is not strictly necessary, but it means that libunwind would consistently have support for compressed .debug_info sections on ARM. This does not impact x86_64. Testing: - Ran an ARM build on Ubuntu 20.04 Change-Id: Iee87b908a5771c2bd7c1a653504a1449d0792280 Reviewed-on: http://gerrit.cloudera.org:8080/19846 Reviewed-by: Alexey Serbin Tested-by: Alexey Serbin --- M CMakeLists.txt M thirdparty/build-thirdparty.sh 2 files changed, 12 insertions(+), 5 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/19846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iee87b908a5771c2bd7c1a653504a1449d0792280 Gerrit-Change-Number: 19846 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3474: Add zlib as dependency of libunwind for ARM
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19846 ) Change subject: KUDU-3474: Add zlib as dependency of libunwind for ARM .. Patch Set 2: Verified+1 Unrelated CI failure (tests were running for too long): * Build timed out (after 50 minutes). Marking the build as failed. Unrelated test failures: * RaftConsensusElectionITest.TestNewLeaderCantResolvePeers * AuthzProviders/TSAuthzTxnOpsITest.BasicOpsOnEmptyTransactions/Ranger due to ranger's fluke (it didn't start): Bad status: Network error: Failed to start the Ranger service: Failed to create Kudu service: Error received from Ranger: : curl error: Couldn't connect to serv er: Failed to connect to 127.18.126.212 port 38787: Connection refused -- To view, visit http://gerrit.cloudera.org:8080/19846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee87b908a5771c2bd7c1a653504a1449d0792280 Gerrit-Change-Number: 19846 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 17 May 2023 05:23:06 + Gerrit-HasComments: No
[kudu-CR] [util] clear OpenSSL error queue on jwt-cpp activity
Alexey Serbin has removed a vote on this change. Change subject: [util] clear OpenSSL error queue on jwt-cpp activity .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/19895 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I1a18a0d78bca0ea32878dc3825523e1bd6ecb019 Gerrit-Change-Number: 19895 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] [util] clear OpenSSL error queue on jwt-cpp activity
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19895 ) Change subject: [util] clear OpenSSL error queue on jwt-cpp activity .. Patch Set 2: Verified+1 unrelated test failure in raft_consensus_election-itest.0 -- To view, visit http://gerrit.cloudera.org:8080/19895 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a18a0d78bca0ea32878dc3825523e1bd6ecb019 Gerrit-Change-Number: 19895 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Wed, 17 May 2023 05:15:06 + Gerrit-HasComments: No
[kudu-CR] [server] remove unused JWT-related flags
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19891 ) Change subject: [server] remove unused JWT-related flags .. [server] remove unused JWT-related flags As it turns out, the following JWT flags aren't used in the code at all: * --jwks_discovery_endpoint_base * --jwt_allow_without_tls * --jwt_validate_signature This patches removes the definitions of the flags and their usage in a few tests. Change-Id: Ib8e688b7c89e24cb5e91f6b6cc89b7ae984b4c35 Reviewed-on: http://gerrit.cloudera.org:8080/19891 Tested-by: Alexey Serbin Reviewed-by: Yingchun Lai --- M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/server/server_base.cc 2 files changed, 0 insertions(+), 19 deletions(-) Approvals: Alexey Serbin: Verified Yingchun Lai: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/19891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib8e688b7c89e24cb5e91f6b6cc89b7ae984b4c35 Gerrit-Change-Number: 19891 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] [server] remove unused JWT-related flags
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19891 ) Change subject: [server] remove unused JWT-related flags .. Patch Set 2: Thank you for review, Yingchun! -- To view, visit http://gerrit.cloudera.org:8080/19891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib8e688b7c89e24cb5e91f6b6cc89b7ae984b4c35 Gerrit-Change-Number: 19891 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Wed, 17 May 2023 05:13:09 + Gerrit-HasComments: No
[kudu-CR] KUDU-3413 [multi-tenancy] update server key for multi-tenancy
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/19622 )
Change subject: KUDU-3413 [multi-tenancy] update server key for multi-tenancy
..
Patch Set 11:
(10 comments)
Thank you for working on the multi-tenant feature.
I apologize for the very late review.
I think it's would be great to
* move simple renamings into a separate changelist
* move the upgrade procedure into a new kudu CLI tool since that way it would
be
** more controllable w.r.t. upgrading the data and handle possible failures
** more practical from keeping the run-time of the tablet servers free of
the code of 'single-use'
Also, please document (at least in the changelist's description) what's the
behaviour of a tablet server of prior version when it run on the data
directories 'converted' into multi-tenant ones.
http://gerrit.cloudera.org:8080/#/c/19622/11//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19622/11//COMMIT_MSG@16
PS11, Line 16: If the flag is enabled, a default tenant will be
: generated using the existing server key info during bootstrap,
the key
: information of both will be exactly the same. At the same time,
: the information of the server key will be preserved.
What happens when a tablet server of prior versions or a tablet server of newer
version but with --enable_multi_tenancy=false is run against such 'upgraded'
data? Will it silently corrupt the data?
http://gerrit.cloudera.org:8080/#/c/19622/11/src/kudu/fs/fs.proto
File src/kudu/fs/fs.proto:
http://gerrit.cloudera.org:8080/#/c/19622/11/src/kudu/fs/fs.proto@47
PS11, Line 47: we have to be compatible with the upgrade of the old
: // version
Please rephrase this. The compatibility isn't something that affects 'us' and
'them', but rather filesystem metadata files between different versions, etc.
Essentially, it would be great to layout the exact terms of what
'compatibility' means here.
http://gerrit.cloudera.org:8080/#/c/19622/11/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
PS11:
Why to have all this code in the runtime of a tablet server when it's is
supposed to be used only once and in rare case if somebody starts using the
multi-tenancy feature?
Wouldn't it be more practical to have the upgrade procedure as a new kudu CLI
tool?
http://gerrit.cloudera.org:8080/#/c/19622/11/src/kudu/fs/fs_manager.cc@513
PS11, Line 513: CHECK(FLAGS_enable_multi_tenancy);
Why to crash here if FLAGS_enable_multi_tenancy is set 'false' even in RELEASE
build? Why not just return Status::IllegalState(), exiting gracefully from
such a situation?
http://gerrit.cloudera.org:8080/#/c/19622/11/src/kudu/fs/key_provider.h
File src/kudu/fs/key_provider.h:
PS11:
Could you please separate this and other simple renaming changes into its own
changelist (before introducing those multi-tenancy extras)?
http://gerrit.cloudera.org:8080/#/c/19622/11/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/19622/11/src/kudu/mini-cluster/external_mini_cluster.cc@1374
PS11, Line 1374: tenant_name
Can a valid 'tenant_name' be empty?
http://gerrit.cloudera.org:8080/#/c/19622/11/src/kudu/mini-cluster/external_mini_cluster.cc@1380
PS11, Line 1380: if (!instance.server_key().empty()) {
:
RETURN_NOT_OK(key_provider_->DecryptEncryptedKey(instance.server_key(),
:
instance.server_key_iv(),
:
instance.server_key_version(),
:
&decrypted_key));
: } else {
: const InstanceMetadataPB::TenantMetadataPB* tenant =
nullptr;
: for (const auto& tdata : instance.tenants()) {
: if (tdata.tenant_name() == tenant_name) {
: tenant = &tdata;
: break;
: }
: }
: if (tenant && !tenant->tenant_key().empty()) {
:
RETURN_NOT_OK(key_provider_->DecryptEncryptedKey(tenant->tenant_key(),
:
tenant->tenant_key_iv(),
:
tenant->tenant_key_version(),
:
&decrypted_key));
: }
: }
I'm not sure this reflects the priority of tenant keys described in the commit
description:
> The priority of tenant key is higher than that of server key, which
> means that if both server key info and tenants info exist in the
> metadata, we will use the tenants.
http://gerrit.cloude
[kudu-CR] [tests] fix flakiness in TestLogCleanupOnStartup
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19871 ) Change subject: [tests] fix flakiness in TestLogCleanupOnStartup .. Patch Set 1: This patch https://gerrit.cloudera.org/c/19842/ is aim to fix this issue too. -- To view, visit http://gerrit.cloudera.org:8080/19871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ee02316fb0850a52251f3740309213b69ca79b8 Gerrit-Change-Number: 19871 Gerrit-PatchSet: 1 Gerrit-Owner: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Wed, 17 May 2023 05:12:02 + Gerrit-HasComments: No
[kudu-CR] [tests] fix flakiness in TestLogCleanupOnStartup
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19871 ) Change subject: [tests] fix flakiness in TestLogCleanupOnStartup .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/19871/1/src/kudu/integration-tests/log-rolling-itest.cc File src/kudu/integration-tests/log-rolling-itest.cc: http://gerrit.cloudera.org:8080/#/c/19871/1/src/kudu/integration-tests/log-rolling-itest.cc@85 PS1, Line 85: SleepFor(MonoDelta::FromSeconds(3)); This will cause this unit test case consume 3 seconds at least, the ASSERT_EVENTUALLY will retry with some time intervals itself, so I guess we can remove this sleep, right? -- To view, visit http://gerrit.cloudera.org:8080/19871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ee02316fb0850a52251f3740309213b69ca79b8 Gerrit-Change-Number: 19871 Gerrit-PatchSet: 1 Gerrit-Owner: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Wed, 17 May 2023 05:09:19 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3413 [multi-tenancy] update server key for multi-tenancy
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19622 ) Change subject: KUDU-3413 [multi-tenancy] update server key for multi-tenancy .. Patch Set 11: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/19622 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e450d73940eb1dbaac6f905a46d6ccd084f15cf Gerrit-Change-Number: 19622 Gerrit-PatchSet: 11 Gerrit-Owner: KeDeng Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Wed, 17 May 2023 04:27:39 + Gerrit-HasComments: No
[kudu-CR] KUDU-3413 [multi-tenancy] update server key for multi-tenancy
Yingchun Lai has removed a vote on this change. Change subject: KUDU-3413 [multi-tenancy] update server key for multi-tenancy .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/19622 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I9e450d73940eb1dbaac6f905a46d6ccd084f15cf Gerrit-Change-Number: 19622 Gerrit-PatchSet: 11 Gerrit-Owner: KeDeng Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du
[kudu-CR] [server] remove unused JWT-related flags
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19891 ) Change subject: [server] remove unused JWT-related flags .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib8e688b7c89e24cb5e91f6b6cc89b7ae984b4c35 Gerrit-Change-Number: 19891 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Wed, 17 May 2023 04:26:51 + Gerrit-HasComments: No
[kudu-CR] KUDU-3475: Update to the most recent sse2neon.h
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19847 ) Change subject: KUDU-3475: Update to the most recent sse2neon.h .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id156a0b2c3984f9cb1032fa7747d7f9ed76c1f8c Gerrit-Change-Number: 19847 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Wed, 17 May 2023 04:24:57 + Gerrit-HasComments: No
[kudu-CR] KUDU-3475: Update to the most recent sse2neon.h
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19847 ) Change subject: KUDU-3475: Update to the most recent sse2neon.h .. Patch Set 4: > Patch Set 4: > > > > > Patch Set 4: > > > > > > > > Thanks to the contribution! > > > > > > > > But I'm curious why not introduce it as a thirdparty? > > > > > > > > > It is from https://github.com/DLTcollab/sse2neon, thanks to it's > > > license is MIT, but I don't think introduce it's source code to > > > Kudu directly is a good idea. > > > > If we want to rework it to make sse2neon.h come from thirdparty, we > > can. sse2neon.h was checked in to Kudu as part of > https://github.com/apache/kudu/commit/74459bd1b3ba311360a8b832873d9e3273dfc83a, > > so this change was upgrading it in place. If we move it to > > thirdparty, we'll need to update the existing #includes. > > I guess it's better to keep this in the main tree in the scope of this patch. > If necessary, we can move this into the thirdparty in a separate patch. That's OK, we can do it in another patch. -- To view, visit http://gerrit.cloudera.org:8080/19847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id156a0b2c3984f9cb1032fa7747d7f9ed76c1f8c Gerrit-Change-Number: 19847 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Wed, 17 May 2023 04:24:48 + Gerrit-HasComments: No
[kudu-CR] KUDU-3474: Add zlib as dependency of libunwind for ARM
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19846 ) Change subject: KUDU-3474: Add zlib as dependency of libunwind for ARM .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee87b908a5771c2bd7c1a653504a1449d0792280 Gerrit-Change-Number: 19846 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 17 May 2023 03:31:19 + Gerrit-HasComments: No
[kudu-CR] KUDU-3475: Update to the most recent sse2neon.h
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19847 ) Change subject: KUDU-3475: Update to the most recent sse2neon.h .. Patch Set 4: Code-Review+2 Thank you for the contribution! -- To view, visit http://gerrit.cloudera.org:8080/19847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id156a0b2c3984f9cb1032fa7747d7f9ed76c1f8c Gerrit-Change-Number: 19847 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Wed, 17 May 2023 03:27:41 + Gerrit-HasComments: No
[kudu-CR] KUDU-3475: Update to the most recent sse2neon.h
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19847 ) Change subject: KUDU-3475: Update to the most recent sse2neon.h .. Patch Set 4: > > > Patch Set 4: > > > > > > Thanks to the contribution! > > > > > > But I'm curious why not introduce it as a thirdparty? > > > > > > It is from https://github.com/DLTcollab/sse2neon, thanks to it's > > license is MIT, but I don't think introduce it's source code to > > Kudu directly is a good idea. > > If we want to rework it to make sse2neon.h come from thirdparty, we > can. sse2neon.h was checked in to Kudu as part of > https://github.com/apache/kudu/commit/74459bd1b3ba311360a8b832873d9e3273dfc83a, > so this change was upgrading it in place. If we move it to > thirdparty, we'll need to update the existing #includes. I guess it's better to keep this in the main tree in the scope of this patch. If necessary, we can move this into the thirdparty in a separate patch. -- To view, visit http://gerrit.cloudera.org:8080/19847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id156a0b2c3984f9cb1032fa7747d7f9ed76c1f8c Gerrit-Change-Number: 19847 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Wed, 17 May 2023 03:26:07 + Gerrit-HasComments: No
[kudu-CR] [rpc] tighten TLS cert requirements for JWT authn
Hello Zoltan Chovan, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19896 to look at the new patch set (#3). Change subject: [rpc] tighten TLS cert requirements for JWT authn .. [rpc] tighten TLS cert requirements for JWT authn This patch addresses a few JWT-related issues in the context of RPC connection negotiation: * At the client side (C++), the negotiating actor requires the server to have a trusted TLS certificate. That's to address one of the TODOs in the code which is very important from the security standpoint. Without verifying the authenticity of the negotiating server-side party, client might send its bearer token to a malicious impostor that would be able to hijack the client's authn token: that would be is a serious security flaw in various real world scenarios. With the stricter requirements introduced, JWT authentication is now available only when the Kudu's IPKI CA cert is in the client's certificate bundle, or Kudu servers are run with TLS certificates that are signed by a reputable CA that in the client's certificate bundle. A test-only --jwt_client_require_trusted_tls_cert flag is added to relax this requirement to abstract away from certificate deployment issues. * At the server side, JWT authn mechanism is advertised to the client only when the server has a CA-signed TLS certificate, so the client at least have an ability to verify the server's certificate. That's similar to the part of the policy used for advertising the TOKEN (Kudu authentication token) mechanism. I updated the existing JWT-related tests (C++) to pass with these changes. I hadn't touched the Java client and the existing tests didn't fail, so I didn't look deeper. Anyways, I was not going to update the corresponding parts of the Java client in this patch. Change-Id: Id2b45227cc4d827b8fab2d9517c09b62135fd757 --- M src/kudu/integration-tests/security-itest.cc M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/server_negotiation.cc 3 files changed, 74 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/19896/3 -- To view, visit http://gerrit.cloudera.org:8080/19896 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id2b45227cc4d827b8fab2d9517c09b62135fd757 Gerrit-Change-Number: 19896 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] [server] remove unused JWT-related flags
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19891 ) Change subject: [server] remove unused JWT-related flags .. Patch Set 2: Verified+1 unrelated test failures (flakiness): * TabletCopyClientBasicTestModes/TabletCopyClientBasicTest.TestDownloadBlockMayFail/1 * AutoRebalancerTest.NextLeaderResumesAutoRebalancing * LogRollingITest.TestLogCleanupOnStartup -- To view, visit http://gerrit.cloudera.org:8080/19891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib8e688b7c89e24cb5e91f6b6cc89b7ae984b4c35 Gerrit-Change-Number: 19891 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Wed, 17 May 2023 02:13:30 + Gerrit-HasComments: No
[kudu-CR] [server] remove unused JWT-related flags
Alexey Serbin has removed a vote on this change. Change subject: [server] remove unused JWT-related flags .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/19891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ib8e688b7c89e24cb5e91f6b6cc89b7ae984b4c35 Gerrit-Change-Number: 19891 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] [util] clear OpenSSL error queue on jwt-cpp activity
Hello Zoltan Chovan, Attila Bukor, Kudu Jenkins, Wenzhe Zhou, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19895 to look at the new patch set (#2). Change subject: [util] clear OpenSSL error queue on jwt-cpp activity .. [util] clear OpenSSL error queue on jwt-cpp activity As it turns out, jwt-cpp doesn't clean up OpenSSL error queue in case of errors. While I think it's a good idea to correct that improper OpenSSL API usage in the jwt-cpp library itself (and I'm going to work on a patch for jwt-cpp library upstream), I updated Kudu's jwt-util code to report on those errors and clean the error queue as an interim solution. I haven't added any tests, but a few tests in a follow-up patch would fail without these changes. This is a follow-up to 5a3d116f302bde07e86bf80c237f8a595d5003b4. Change-Id: I1a18a0d78bca0ea32878dc3825523e1bd6ecb019 --- M python/kudu/tests/test_client.py M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc 3 files changed, 47 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/19895/2 -- To view, visit http://gerrit.cloudera.org:8080/19895 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a18a0d78bca0ea32878dc3825523e1bd6ecb019 Gerrit-Change-Number: 19895 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] KUDU-3475: Update to the most recent sse2neon.h
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19847 ) Change subject: KUDU-3475: Update to the most recent sse2neon.h .. Patch Set 4: > > Patch Set 4: > > > > Thanks to the contribution! > > > > But I'm curious why not introduce it as a thirdparty? > > > It is from https://github.com/DLTcollab/sse2neon, thanks to it's > license is MIT, but I don't think introduce it's source code to > Kudu directly is a good idea. If we want to rework it to make sse2neon.h come from thirdparty, we can. sse2neon.h was checked in to Kudu as part of https://github.com/apache/kudu/commit/74459bd1b3ba311360a8b832873d9e3273dfc83a, so this change was upgrading it in place. If we move it to thirdparty, we'll need to update the existing #includes. -- To view, visit http://gerrit.cloudera.org:8080/19847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id156a0b2c3984f9cb1032fa7747d7f9ed76c1f8c Gerrit-Change-Number: 19847 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Tue, 16 May 2023 23:39:32 + Gerrit-HasComments: No
[kudu-CR] KUDU-3474: Add zlib as dependency of libunwind for ARM
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19846 Change subject: KUDU-3474: Add zlib as dependency of libunwind for ARM .. KUDU-3474: Add zlib as dependency of libunwind for ARM On ARM, libunwind includes support for compressed .debug_info sections if zlib is available during compilation. This means that libunwind can require zlib at link time, but only for ARM architecture. This modifies the build to add a zlib dependency for libunwind if the architecture is ARM. It also reorders the thirdparty build so that zlib is built before libunwind. This is not strictly necessary, but it means that libunwind would consistently have support for compressed .debug_info sections on ARM. This does not impact x86_64. Testing: - Ran an ARM build on Ubuntu 20.04 Change-Id: Iee87b908a5771c2bd7c1a653504a1449d0792280 --- M CMakeLists.txt M thirdparty/build-thirdparty.sh 2 files changed, 12 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/19846/2 -- To view, visit http://gerrit.cloudera.org:8080/19846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iee87b908a5771c2bd7c1a653504a1449d0792280 Gerrit-Change-Number: 19846 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins
[kudu-CR] KUDU-1945 Add Python example for non-unique PK
Wenzhe Zhou has posted comments on this change. (
http://gerrit.cloudera.org:8080/19874 )
Change subject: KUDU-1945 Add Python example for non-unique PK
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19874/3/examples/python/basic-python-example/non_unique_primary_key.py
File examples/python/basic-python-example/non_unique_primary_key.py:
http://gerrit.cloudera.org:8080/#/c/19874/3/examples/python/basic-python-example/non_unique_primary_key.py@142
PS3, Line 142: Updated row(s) WHERE non_unique_key = {0} AND int_val = {1} to
int_val = {2}'\
It's better to change to: 'Updated row(s) int_val = {2} WHERE non_unique_key =
{0} AND int_val = {1}'
--
To view, visit http://gerrit.cloudera.org:8080/19874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dd862c4f26b3d79ec268727363fae3426135f52
Gerrit-Change-Number: 19874
Gerrit-PatchSet: 3
Gerrit-Owner: Marton Greber
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Chovan
Gerrit-Comment-Date: Tue, 16 May 2023 20:42:49 +
Gerrit-HasComments: Yes
[kudu-CR] [util] clear OpenSSL error queue on jwt-cpp activity
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19895 ) Change subject: [util] clear OpenSSL error queue on jwt-cpp activity .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/19895 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a18a0d78bca0ea32878dc3825523e1bd6ecb019 Gerrit-Change-Number: 19895 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Tue, 16 May 2023 20:16:40 + Gerrit-HasComments: No
[kudu-CR] [util] clear OpenSSL error queue on jwt-cpp activity
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/19895 ) Change subject: [util] clear OpenSSL error queue on jwt-cpp activity .. Patch Set 1: Code-Review+1 Looks good to me, but it seems that some python tests are failing. Maybe some error message assertions need to be updated there as well? -- To view, visit http://gerrit.cloudera.org:8080/19895 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a18a0d78bca0ea32878dc3825523e1bd6ecb019 Gerrit-Change-Number: 19895 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Tue, 16 May 2023 18:43:34 + Gerrit-HasComments: No
[kudu-CR] [rpc] tighten TLS cert requirements for JWT authn
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19896 Change subject: [rpc] tighten TLS cert requirements for JWT authn .. [rpc] tighten TLS cert requirements for JWT authn This patch addresses a few JWT-related issues in the context of RPC connection negotiation: * At the client side (C++), the negotiating actor requires the server to have a trusted TLS certificate. That's to address one of the TODOs in the code which is very important from the security standpoint. Without verifying the authenticity of the negotiating server-side party, client might send its bearer token to a malicious impostor that would be able to hijack the client's authn token: that would be is a serious security flaw in various real world scenarios. With the stricter requirements introduced, JWT authentication is now available only when the Kudu's IPKI CA cert is in the client's certificate bundle, or Kudu servers are run with TLS certificates that are signed by a reputable CA that in the client's certificate bundle. A test-only --jwt_client_require_trusted_tls_cert flag is added to relax this requirement to abstract away from certificate deployment issues. * At the server side, JWT authn mechanism is advertised to the client only when the server has a CA-signed TLS certificate, so the client at least have an ability to verify the server's certificate. That's similar to the part of the policy used for advertising the TOKEN (Kudu authentication token) mechanism. I updated the existing JWT-related tests (C++) to pass with these changes. I haven't touched the Java client and the existing tests haven't failed, so I haven't looked deeper. Anyways, I'm not going to take care of the Java client-side code in this patch. Change-Id: Id2b45227cc4d827b8fab2d9517c09b62135fd757 --- M src/kudu/integration-tests/security-itest.cc M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/server_negotiation.cc 3 files changed, 74 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/19896/1 -- To view, visit http://gerrit.cloudera.org:8080/19896 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id2b45227cc4d827b8fab2d9517c09b62135fd757 Gerrit-Change-Number: 19896 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] [util] clear OpenSSL error queue on jwt-cpp activity
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19895 Change subject: [util] clear OpenSSL error queue on jwt-cpp activity .. [util] clear OpenSSL error queue on jwt-cpp activity As it turns out, jwt-cpp doesn't clean up OpenSSL error queue in case of errors. While I think it's a good idea to correct that improper OpenSSL API usage in the jwt-cpp library itself (and I'm going to work on a patch for jwt-cpp library upstream), I updated Kudu's jwt-util code to report on those errors and clean the error queue as an interim solution. I haven't added any tests, but a few tests in a follow-up patch would fail without these changes. This is a follow-up to 5a3d116f302bde07e86bf80c237f8a595d5003b4. Change-Id: I1a18a0d78bca0ea32878dc3825523e1bd6ecb019 --- M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc 2 files changed, 50 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/19895/1 -- To view, visit http://gerrit.cloudera.org:8080/19895 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1a18a0d78bca0ea32878dc3825523e1bd6ecb019 Gerrit-Change-Number: 19895 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] [cpp-client] KUDU-3455 Reduce space complexity and speed up hash partition pruning for in-list predicate
Yingchun Lai has posted comments on this change. (
http://gerrit.cloudera.org:8080/19794 )
Change subject: [cpp-client] KUDU-3455 Reduce space complexity and speed up
hash partition pruning for in-list predicate
..
Patch Set 7:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@9
PS7, Line 9: https://gerrit.cloudera.org/c/19568/
It would be better to use the related commit hash i.e. b69dbeb instead, then it
would be more convenient to show the info by using "git log b69dbeb".
http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@16
PS7, Line 16: https://gerrit.cloudera.org/c/19568/
Same.
http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@21
PS7, Line 21: columns
nit: keys
http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@21
PS7, Line 21: The
nit: the
http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@22
PS7, Line 22: bigger
nit: longger?
http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@24
PS7, Line 24: Using
nit: using
http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner-test.cc
File src/kudu/common/partition_pruner-test.cc:
http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner-test.cc@97
PS7, Line 97: std::vector
nit: vector
http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner.cc
File src/kudu/common/partition_pruner.cc:
http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner.cc@186
PS7, Line 186: vector hash_bucket_bitset(hash_dimension.num_buckets,
false);
nit: Move this line to L207 where is closer to the place it's used ?
http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner.cc@217
PS7, Line 217: bool immediately_stop = true;
nit: adds DCHECK to check predicate_values_picked and hash_bucket_bitset will
not be nullptr?
http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner.cc@219
PS7, Line 219: immediately_stop &= b;
: }
: if (immediately_stop) {
: return;
: }
Why not return immediately once find a true value in the loop?
http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner.cc@225
PS7, Line 225: CHECK_EQ(hash_dimension.column_ids.size(),
predicate_values_list.size());
How about using DCHECK_EQ which is take effect only in DEBUG version? The
release version which is usually used in product environment will not be
effected.
--
To view, visit http://gerrit.cloudera.org:8080/19794
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie4bea5c10b4ac2c62b85625fe9d2a33ceb4fb2e9
Gerrit-Change-Number: 19794
Gerrit-PatchSet: 7
Gerrit-Owner: Yuqi Du
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Ashwani Raina
Gerrit-Reviewer: KeDeng
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai
Gerrit-Reviewer: Yuqi Du
Gerrit-Comment-Date: Tue, 16 May 2023 15:46:39 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3413 [multi-tenancy] update server key for multi-tenancy
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19622 ) Change subject: KUDU-3413 [multi-tenancy] update server key for multi-tenancy .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19622 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e450d73940eb1dbaac6f905a46d6ccd084f15cf Gerrit-Change-Number: 19622 Gerrit-PatchSet: 11 Gerrit-Owner: KeDeng Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Tue, 16 May 2023 10:59:24 + Gerrit-HasComments: No
[kudu-CR] WIP WIP kudu scheduler compaction tasks
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19325 to look at the new patch set (#5). Change subject: WIP WIP kudu scheduler_compaction tasks .. WIP WIP kudu scheduler_compaction tasks This patch will support trigger tablet compaction task by administrators. Change-Id: I6a792751cb5bd9c52109a0bc08545e3ce47a8f06 --- M src/kudu/common/wire_protocol.proto M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_mm_ops.h M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tablet/tablet_replica_mm_ops.h M src/kudu/tools/tool_action_table.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/tserver_admin.proto M src/kudu/util/maintenance_manager-test.cc M src/kudu/util/maintenance_manager.cc M src/kudu/util/maintenance_manager.h 14 files changed, 322 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/19325/5 -- To view, visit http://gerrit.cloudera.org:8080/19325 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6a792751cb5bd9c52109a0bc08545e3ce47a8f06 Gerrit-Change-Number: 19325 Gerrit-PatchSet: 5 Gerrit-Owner: Yuqi Du Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [KUDU-3452] Make validate tablet creating task not affected
Yifan Zhang has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19594 ) Change subject: [KUDU-3452] Make validate tablet creating task not affected .. [KUDU-3452] Make validate tablet creating task not affected Currently, creating a table with RF=n when the number of healthy tservers is less than n will get stuck. Because catalog manager creates tablets for it will fail and retry continuously. At the same time, creating a table with RF=m also will get stuck even if there are more than m healthy tservers. Because catalog manager will return when finds a tablet-creating task failed and will not try to select replicas for other PREPARING tablets. For example, creating a three replicas table times out when one of three tablet servers becomes unavailable. After that, creating a two-replicas table also will timeout even if there are enough tablet servers to place its replicas. The validate two-replicas table-creating task will be affected by invalidate three-replicas table-creating task. This patch fixes this problem. If a task of creating tablet fail, it will not return immediately, but let other tasks of creating other tablets keep on running. Change-Id: I64668651d0e8f58b92cfb841bdb20617de6776f9 Reviewed-on: http://gerrit.cloudera.org:8080/19594 Reviewed-by: Yifan Zhang Tested-by: Yifan Zhang --- M src/kudu/integration-tests/create-table-itest.cc M src/kudu/master/catalog_manager.cc 2 files changed, 109 insertions(+), 5 deletions(-) Approvals: Yifan Zhang: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/19594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I64668651d0e8f58b92cfb841bdb20617de6776f9 Gerrit-Change-Number: 19594 Gerrit-PatchSet: 25 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du
[kudu-CR] [KUDU-3452] Make validate tablet creating task not affected
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19594 ) Change subject: [KUDU-3452] Make validate tablet creating task not affected .. Patch Set 24: Verified+1 Unrelated test failures in log-rolling-itest (RELEASE mode). -- To view, visit http://gerrit.cloudera.org:8080/19594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64668651d0e8f58b92cfb841bdb20617de6776f9 Gerrit-Change-Number: 19594 Gerrit-PatchSet: 24 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Tue, 16 May 2023 10:20:45 + Gerrit-HasComments: No
[kudu-CR] [KUDU-3452] Make validate tablet creating task not affected
Yifan Zhang has removed a vote on this change. Change subject: [KUDU-3452] Make validate tablet creating task not affected .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/19594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I64668651d0e8f58b92cfb841bdb20617de6776f9 Gerrit-Change-Number: 19594 Gerrit-PatchSet: 24 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du
[kudu-CR] [KUDU-3452] Make validate tablet creating task not affected
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19594 ) Change subject: [KUDU-3452] Make validate tablet creating task not affected .. Patch Set 24: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64668651d0e8f58b92cfb841bdb20617de6776f9 Gerrit-Change-Number: 19594 Gerrit-PatchSet: 24 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Tue, 16 May 2023 09:40:24 + Gerrit-HasComments: No
[kudu-CR] [KUDU-3452] Make validate tablet creating task not affected
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19594 ) Change subject: [KUDU-3452] Make validate tablet creating task not affected .. Patch Set 24: (2 comments) http://gerrit.cloudera.org:8080/#/c/19594/23/src/kudu/integration-tests/create-table-itest.cc File src/kudu/integration-tests/create-table-itest.cc: http://gerrit.cloudera.org:8080/#/c/19594/23/src/kudu/integration-tests/create-table-itest.cc@782 PS23, Line 782: const > nit: What about using 'constexpr const'? Same as elsewhere. Done http://gerrit.cloudera.org:8080/#/c/19594/23/src/kudu/integration-tests/create-table-itest.cc@801 PS23, Line 801: // Wait until the tablet server become unavailable for replicas placement. > nit: Wait until the tablet server become unavailable for replicas placement Done -- To view, visit http://gerrit.cloudera.org:8080/19594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64668651d0e8f58b92cfb841bdb20617de6776f9 Gerrit-Change-Number: 19594 Gerrit-PatchSet: 24 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Tue, 16 May 2023 09:20:15 + Gerrit-HasComments: Yes
[kudu-CR] [KUDU-3452] Make validate tablet creating task not affected
Hello Tidy Bot, Alexey Serbin, Yuqi Du, Yingchun Lai, Yifan Zhang, Kudu Jenkins, KeDeng, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19594 to look at the new patch set (#24). Change subject: [KUDU-3452] Make validate tablet creating task not affected .. [KUDU-3452] Make validate tablet creating task not affected Currently, creating a table with RF=n when the number of healthy tservers is less than n will get stuck. Because catalog manager creates tablets for it will fail and retry continuously. At the same time, creating a table with RF=m also will get stuck even if there are more than m healthy tservers. Because catalog manager will return when finds a tablet-creating task failed and will not try to select replicas for other PREPARING tablets. For example, creating a three replicas table times out when one of three tablet servers becomes unavailable. After that, creating a two-replicas table also will timeout even if there are enough tablet servers to place its replicas. The validate two-replicas table-creating task will be affected by invalidate three-replicas table-creating task. This patch fixes this problem. If a task of creating tablet fail, it will not return immediately, but let other tasks of creating other tablets keep on running. Change-Id: I64668651d0e8f58b92cfb841bdb20617de6776f9 --- M src/kudu/integration-tests/create-table-itest.cc M src/kudu/master/catalog_manager.cc 2 files changed, 109 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/19594/24 -- To view, visit http://gerrit.cloudera.org:8080/19594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I64668651d0e8f58b92cfb841bdb20617de6776f9 Gerrit-Change-Number: 19594 Gerrit-PatchSet: 24 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du
[kudu-CR] [KUDU-3452] Make validate tablet creating task not affected
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19594 ) Change subject: [KUDU-3452] Make validate tablet creating task not affected .. Patch Set 23: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/19594/23/src/kudu/integration-tests/create-table-itest.cc File src/kudu/integration-tests/create-table-itest.cc: http://gerrit.cloudera.org:8080/#/c/19594/23/src/kudu/integration-tests/create-table-itest.cc@782 PS23, Line 782: const nit: What about using 'constexpr const'? Same as elsewhere. http://gerrit.cloudera.org:8080/#/c/19594/23/src/kudu/integration-tests/create-table-itest.cc@801 PS23, Line 801: // Wait for tablet server heartbeat timeout. nit: Wait until the tablet server become unavailable for replicas placement. -- To view, visit http://gerrit.cloudera.org:8080/19594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64668651d0e8f58b92cfb841bdb20617de6776f9 Gerrit-Change-Number: 19594 Gerrit-PatchSet: 23 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Tue, 16 May 2023 08:57:38 + Gerrit-HasComments: Yes
