[kudu-CR] [Java] Upgrade dependencies
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16874 ) Change subject: [Java] Upgrade dependencies .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib4b319a67f629bcb60ae54187cee1c3257d6e1a2 Gerrit-Change-Number: 16874 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 04 Jan 2021 14:31:02 + Gerrit-HasComments: No
[kudu-CR] [docker] Update the supported base images
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16877 ) Change subject: [docker] Update the supported base images .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I80b90f77f803a503f63f616a61080dcde4ee35a5 Gerrit-Change-Number: 16877 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 04 Jan 2021 14:31:24 + Gerrit-HasComments: No
[kudu-CR] [docs] RPC cancellation is now implemented
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16918 ) Change subject: [docs] RPC cancellation is now implemented .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I335b3c0fd965896aff7e1625fd01ae564c050b1a Gerrit-Change-Number: 16918 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 04 Jan 2021 15:43:49 + Gerrit-HasComments: No
[kudu-CR] [docs] RPC cancellation is now implemented
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16918 ) Change subject: [docs] RPC cancellation is now implemented .. [docs] RPC cancellation is now implemented This patch updates the documentation on the RPC design in rpc.md, refreshing the note on the RPC cancellation. Since KUDU-2065 has been addressed, it's now possible to cancel a remote call using the RpcController::Cancel() method (follow up JIRAs KUDU-2334 and KUDU-2011 are still open, but the functionality for RPC cancellation is there). Change-Id: I335b3c0fd965896aff7e1625fd01ae564c050b1a Reviewed-on: http://gerrit.cloudera.org:8080/16918 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M docs/design-docs/rpc.md 1 file changed, 8 insertions(+), 8 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I335b3c0fd965896aff7e1625fd01ae564c050b1a Gerrit-Change-Number: 16918 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [rpc] update ServiceIf::service name() signature
Hello Mahesh Reddy, Tidy Bot, Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16916 to look at the new patch set (#3). Change subject: [rpc] update ServiceIf::service_name() signature .. [rpc] update ServiceIf::service_name() signature This patch changes the signature of the ServiceIf::service_name() method to return 'const std::string&' instead of 'std::string' and updates a few related call sites accordingly. In addition, this patch removes unused ServiceIf::methods_by_name() accessor method and makes ServiceIf::ParseParam() and ServiceIf::RespondBadMethod() methods static. The ServiceIf::service_name() method isn't in any hot path but it's used in ServicePool::RejectTooBusy(). The latter is usually called under high memory pressure when it's preferred to avoid memory allocations at least because of the lock contention in the tcmalloc library. I also did a few formatting changes in protoc-gen-krpc.cc to improve the readability of the auto-generated code since I looked at that code recently. Change-Id: Ifa7fc425cf789fa31e99642061b6474b0effd489 --- M src/kudu/rpc/protoc-gen-krpc.cc M src/kudu/rpc/rpc-test-base.h M src/kudu/rpc/service_if.cc M src/kudu/rpc/service_if.h M src/kudu/rpc/service_pool.cc M src/kudu/rpc/service_pool.h M src/kudu/server/rpc_server.cc 7 files changed, 278 insertions(+), 271 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/16916/3 -- To view, visit http://gerrit.cloudera.org:8080/16916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifa7fc425cf789fa31e99642061b6474b0effd489 Gerrit-Change-Number: 16916 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [rpc] update ServiceIf::service name() signature
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16916 )
Change subject: [rpc] update ServiceIf::service_name() signature
..
Patch Set 3:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@59
PS3, Line 59: using kudu::rpc_test::AddRequestPB;
> warning: using declarations in the global namespace in headers are prohibit
I'm not going to address this: this is test-only code and this header is used
only in a few places. Updating this code is out of the scope of the patch.
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@60
PS3, Line 60: using kudu::rpc_test::AddResponsePB;
> warning: using declarations in the global namespace in headers are prohibit
ditto
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@61
PS3, Line 61: using kudu::rpc_test::CalculatorError;
> warning: using declarations in the global namespace in headers are prohibit
ditto
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@83
PS3, Line 83: using kudu::rpc_test_diff_package::ReqDiffPackagePB;
> warning: using declarations in the global namespace in headers are prohibit
ditto
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@84
PS3, Line 84: using kudu::rpc_test_diff_package::RespDiffPackagePB;
> warning: using declarations in the global namespace in headers are prohibit
ditto
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@134
PS3, Line 134: void DoAdd(InboundCall *incoming) {
> warning: method 'DoAdd' can be made static [readability-convert-member-func
ditto
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@402
PS3, Line 402: const std::string GenericCalculatorService::kFullServiceName =
"kudu.rpc.GenericCalculatorService";
> warning: variable 'kFullServiceName' defined in a header file; variable def
ditto
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@403
PS3, Line 403: const char *GenericCalculatorService::kAddMethodName = "Add";
> warning: variable 'kAddMethodName' defined in a header file; variable defin
ditto
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@404
PS3, Line 404: const char *GenericCalculatorService::kSleepMethodName = "Sleep";
> warning: variable 'kSleepMethodName' defined in a header file; variable def
ditto
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@405
PS3, Line 405: const char
*GenericCalculatorService::kSleepWithSidecarMethodName = "SleepWithSidecar";
> warning: variable 'kSleepWithSidecarMethodName' defined in a header file; v
ditto
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/service_if.cc
File src/kudu/rpc/service_if.cc:
http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/service_if.cc@149
PS3, Line 149: GeneratedServiceIf::GeneratedServiceIf(const
scoped_refptr& tracker)
> warning: pass by value and use std::move [modernize-pass-by-value]
I'm not going to address this TidyBot comment since the signatures of the rest
of involved methods do not pass this by value.
--
To view, visit http://gerrit.cloudera.org:8080/16916
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7fc425cf789fa31e99642061b6474b0effd489
Gerrit-Change-Number: 16916
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 04 Jan 2021 17:14:08 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3225 Fixes error 'vld1q u8 x4' was not declared
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16907 ) Change subject: KUDU-3225 Fixes error 'vld1q_u8_x4' was not declared .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/16907/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16907/1//COMMIT_MSG@10 PS1, Line 10: always I might be missing something, but it seems the updated code introduces vld1q_u8_x4 only for non-clang case, no? http://gerrit.cloudera.org:8080/#/c/16907/1/src/kudu/util/sse2neon.h File src/kudu/util/sse2neon.h: http://gerrit.cloudera.org:8080/#/c/16907/1/src/kudu/util/sse2neon.h@183 PS1, Line 183: __aarch64__ I'm curious: would this compile with GCC 10 on ARM? It seems this should be defined only for GCC 9 and older. At least, that's so at https://github.com/DLTcollab/sse2neon/blob/471743745dd09798fb7df87a9b7f31f543aa31da/sse2neon.h#L303-L316 -- To view, visit http://gerrit.cloudera.org:8080/16907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2538e6ca321e89edb46d54f620b305b8e005f173 Gerrit-Change-Number: 16907 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 04 Jan 2021 17:28:44 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Upgrade dependencies
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16874 to look at the new patch set (#2). Change subject: [Java] Upgrade dependencies .. [Java] Upgrade dependencies Upgrades the Java dependencies and Gradle versions. Major version upgrades: - guava 29.0-jre -> 30.1-jre - scopt 3.7.1 -> 4.0.0 Minor version upgrades: - jmh 1.26 -> 1.27 - log4j 2.13.3 -> 2.14.0 - micrometer 1.5.5 -> 1.6.2 - mockito 3.5.13 -> 3.6.28 - protobuf 3.13.0 -> 3.14.0 - yetus 0.12.0 -> 0.13.0 Maintenance version upgrades: - jetty 9.4.32.v20200930 -> 9.4.35.v20201120 - netty 4.1.52.Final -> 4.1.56.Final - scalatest 3.2.2 -> 3.2.3 Gradle upgrades: - gradle 6.6.1 -> 6.7.1 - gradle-versions-plugin 0.33.0 -> 0.36.0 - gradle-protobuf-plugin 0.8.13 -> 0.8.14 - nebula-clojure-plugin 9.4.1 -> 9.4.2 - spotbugs-gradle-plugin 4.5.0 -> 4.6.0 - gradle-error-prone-plugin 1.2.1 -> 1.3.0 - gradle-animal-sniffer-plugin 1.5.1 -> 1.5.2 - jmh-gradle-plugin 0.5.1 -> 0.5.2 Change-Id: Ib4b319a67f629bcb60ae54187cee1c3257d6e1a2 --- M java/buildSrc/build.gradle M java/gradle/dependencies.gradle M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestOptions.scala M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java 4 files changed, 26 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/16874/2 -- To view, visit http://gerrit.cloudera.org:8080/16874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib4b319a67f629bcb60ae54187cee1c3257d6e1a2 Gerrit-Change-Number: 16874 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [Java] Upgrade dependencies
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16874 ) Change subject: [Java] Upgrade dependencies .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16874/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java: http://gerrit.cloudera.org:8080/#/c/16874/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java@425 PS1, Line 425: an > nit: drop Done -- To view, visit http://gerrit.cloudera.org:8080/16874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib4b319a67f629bcb60ae54187cee1c3257d6e1a2 Gerrit-Change-Number: 16874 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 04 Jan 2021 18:38:04 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Upgrade dependencies
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16874 ) Change subject: [Java] Upgrade dependencies .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib4b319a67f629bcb60ae54187cee1c3257d6e1a2 Gerrit-Change-Number: 16874 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 04 Jan 2021 18:54:32 + Gerrit-HasComments: No
[kudu-CR] [rpc] update ServiceIf::service name() signature
Alexey Serbin has removed a vote on this change. Change subject: [rpc] update ServiceIf::service_name() signature .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ifa7fc425cf789fa31e99642061b6474b0effd489 Gerrit-Change-Number: 16916 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [rpc] update ServiceIf::service name() signature
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16916 ) Change subject: [rpc] update ServiceIf::service_name() signature .. Patch Set 3: Verified+1 unrelated test failure: src/kudu/integration-tests/disk_failure-itest.cc:289: Failure Expected: num_failed Which is: 10 To be equal to: failed_on_ts Which is: 9 src/kudu/util/test_util.cc:336: Failure Failed Timed out waiting for assertion to pass. src/kudu/integration-tests/disk_failure-itest.cc:330: Failure Expected: WaitForFailedTablets(error_ts, kNumTablets) doesn't generate new fatal failures in the current thread. Actual: it does. -- To view, visit http://gerrit.cloudera.org:8080/16916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifa7fc425cf789fa31e99642061b6474b0effd489 Gerrit-Change-Number: 16916 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 04 Jan 2021 20:39:10 + Gerrit-HasComments: No
[kudu-CR] [Java] Upgrade dependencies
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16874 )
Change subject: [Java] Upgrade dependencies
..
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16874/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java:
http://gerrit.cloudera.org:8080/#/c/16874/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java@427
PS2, Line 427: while (((ByteBuf)
embedder.outboundMessages().peek()).readableBytes() == 0) {
: embedder.readOutbound();
: }
Just curious: is this change due to the fact that with the newer version of
some related packages now there are multiple empty messages, while with prior
versions there could be at most one empty message?
--
To view, visit http://gerrit.cloudera.org:8080/16874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4b319a67f629bcb60ae54187cee1c3257d6e1a2
Gerrit-Change-Number: 16874
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 04 Jan 2021 22:44:33 +
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-2671: Range specific hashing at table creation time.
Hello Kudu Jenkins, Andrew Wong, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16859 to look at the new patch set (#11). Change subject: [master] KUDU-2671: Range specific hashing at table creation time. .. [master] KUDU-2671: Range specific hashing at table creation time. This patch updates CreateTableRequestPB to allow different hash schemas to be defined per range at table creation time. This new field is appropriately decoded in catalog_manager.cc. While this patch handles the logic for creating the correct partitions, it does not update the metadata for either the table or tablets. The new per-range schemas will need to be added to the table metadata in a following patch. The changes to kudu/common include some refactoring and putting functions back into an anonymous namespace. Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d --- M src/kudu/common/partition-test.cc M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/integration-tests/table_locations-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master.proto 7 files changed, 254 insertions(+), 47 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/16859/11 -- To view, visit http://gerrit.cloudera.org:8080/16859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d Gerrit-Change-Number: 16859 Gerrit-PatchSet: 11 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy
[kudu-CR] [master] KUDU-2671: Range specific hashing at table creation time.
Mahesh Reddy has posted comments on this change. (
http://gerrit.cloudera.org:8080/16859 )
Change subject: [master] KUDU-2671: Range specific hashing at table creation
time.
..
Patch Set 11:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16859/10/src/kudu/common/partition-test.cc
File src/kudu/common/partition-test.cc:
http://gerrit.cloudera.org:8080/#/c/16859/10/src/kudu/common/partition-test.cc@1148
PS10, Line 1148:
> Now that there's no per-range hash partitioning in this test, does this tes
I meant to delete this since it's no longer relevant, thanks for pointing it
out.
http://gerrit.cloudera.org:8080/#/c/16859/2/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/16859/2/src/kudu/common/partition.cc@418
PS2, Line 418: if (!range_hash_schemas.empty()) {
: if (!split_rows.empty()) {
: return Status::InvalidArgument("Both 'split_rows' and
'range_hash_schemas' cannot be "
: "populated at the same
time.");
: }
: if (range_bounds.size() != range_hash_schemas.size()) {
:
> SGTM, thanks for the update.
Ack
--
To view, visit http://gerrit.cloudera.org:8080/16859
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d
Gerrit-Change-Number: 16859
Gerrit-PatchSet: 11
Gerrit-Owner: Mahesh Reddy
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Comment-Date: Tue, 05 Jan 2021 00:22:56 +
Gerrit-HasComments: Yes
[kudu-CR] [rpc] update ServiceIf::service name() signature
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16916 ) Change subject: [rpc] update ServiceIf::service_name() signature .. Patch Set 3: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/service_if.h File src/kudu/rpc/service_if.h: http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/service_if.h@118 PS3, Line 118: explicit GeneratedServiceIf(const scoped_refptr& tracker); Not quite sure I understand the reasoning/context behind adding this, everything else in the patch lgtm. -- To view, visit http://gerrit.cloudera.org:8080/16916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifa7fc425cf789fa31e99642061b6474b0effd489 Gerrit-Change-Number: 16916 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 05 Jan 2021 02:00:43 + Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-2671: Range specific hashing at table creation time.
Andrew Wong has removed a vote on this change. Change subject: [master] KUDU-2671: Range specific hashing at table creation time. .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d Gerrit-Change-Number: 16859 Gerrit-PatchSet: 11 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy
[kudu-CR] [master] KUDU-2671: Range specific hashing at table creation time.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16859 ) Change subject: [master] KUDU-2671: Range specific hashing at table creation time. .. Patch Set 11: Verified+1 Code-Review+2 Test failure was unrelated. -- To view, visit http://gerrit.cloudera.org:8080/16859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d Gerrit-Change-Number: 16859 Gerrit-PatchSet: 11 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Tue, 05 Jan 2021 02:42:58 + Gerrit-HasComments: No
[kudu-CR] [master] KUDU-2671: Range specific hashing at table creation time.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16859 ) Change subject: [master] KUDU-2671: Range specific hashing at table creation time. .. [master] KUDU-2671: Range specific hashing at table creation time. This patch updates CreateTableRequestPB to allow different hash schemas to be defined per range at table creation time. This new field is appropriately decoded in catalog_manager.cc. While this patch handles the logic for creating the correct partitions, it does not update the metadata for either the table or tablets. The new per-range schemas will need to be added to the table metadata in a following patch. The changes to kudu/common include some refactoring and putting functions back into an anonymous namespace. Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d Reviewed-on: http://gerrit.cloudera.org:8080/16859 Reviewed-by: Andrew Wong Tested-by: Andrew Wong --- M src/kudu/common/partition-test.cc M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/integration-tests/table_locations-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master.proto 7 files changed, 254 insertions(+), 47 deletions(-) Approvals: Andrew Wong: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/16859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d Gerrit-Change-Number: 16859 Gerrit-PatchSet: 12 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy
[kudu-CR] [rpc] update ServiceIf::service name() signature
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16916 ) Change subject: [rpc] update ServiceIf::service_name() signature .. Patch Set 3: (1 comment) Thank you for the review! http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/service_if.h File src/kudu/rpc/service_if.h: http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/service_if.h@118 PS3, Line 118: explicit GeneratedServiceIf(const scoped_refptr& tracker); > Not quite sure I understand the reasoning/context behind adding this, every Ah, that's to use initializer-list style syntax in the constructor of the auto-generated service skeleton class in https://gerrit.cloudera.org/#/c/16916/3/src/kudu/rpc/protoc-gen-krpc.cc@486 Prior to this, it was necessary to have an assignment in the constructor of the service skeleton class after the field was already initialized in the constructor of the base (i.e. GeneratedServiceIf) class. -- To view, visit http://gerrit.cloudera.org:8080/16916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifa7fc425cf789fa31e99642061b6474b0effd489 Gerrit-Change-Number: 16916 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 05 Jan 2021 02:44:50 + Gerrit-HasComments: Yes
[kudu-CR] [rpc] update ServiceIf::service name() signature
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16916 ) Change subject: [rpc] update ServiceIf::service_name() signature .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/service_if.h File src/kudu/rpc/service_if.h: http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/service_if.h@118 PS3, Line 118: explicit GeneratedServiceIf(const scoped_refptr& tracker); > Ah, that's to use initializer-list style syntax in the constructor of the a I see, thanks for letting me know. -- To view, visit http://gerrit.cloudera.org:8080/16916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifa7fc425cf789fa31e99642061b6474b0effd489 Gerrit-Change-Number: 16916 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 05 Jan 2021 03:13:07 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3225 Fixes error 'vld1q u8 x4' was not declared
huangtianhua...@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/16907 ) Change subject: KUDU-3225 Fixes error 'vld1q_u8_x4' was not declared .. Patch Set 1: I might be missing something, but it seems the updated code introduces vld1q_u8_x4 only for non-clang case, no? --- Yes, clang has defined the type 'vld1q_u8_x4'. -- To view, visit http://gerrit.cloudera.org:8080/16907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2538e6ca321e89edb46d54f620b305b8e005f173 Gerrit-Change-Number: 16907 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jan 2021 04:18:11 + Gerrit-HasComments: No
[kudu-CR] KUDU-3225 Fixes error 'vld1q u8 x4' was not declared
huangtianhua...@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/16907 ) Change subject: KUDU-3225 Fixes error 'vld1q_u8_x4' was not declared .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16907/1/src/kudu/util/sse2neon.h File src/kudu/util/sse2neon.h: http://gerrit.cloudera.org:8080/#/c/16907/1/src/kudu/util/sse2neon.h@183 PS1, Line 183: __aarch64__ > I'm curious: would this compile with GCC 10 on ARM? GCC does not implement the vld1q_u8_x4() intrinsic, so maybe we should follow the community solution to modify to __GNUC__ <= 9, see https://github.com/DLTcollab/sse2neon/pull/31/files -- To view, visit http://gerrit.cloudera.org:8080/16907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2538e6ca321e89edb46d54f620b305b8e005f173 Gerrit-Change-Number: 16907 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jan 2021 06:29:29 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3225 Fixes error 'vld1q u8 x4' was not declared
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16907 ) Change subject: KUDU-3225 Fixes error 'vld1q_u8_x4' was not declared .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16907/1/src/kudu/util/sse2neon.h File src/kudu/util/sse2neon.h: http://gerrit.cloudera.org:8080/#/c/16907/1/src/kudu/util/sse2neon.h@183 PS1, Line 183: __aarch64__ > GCC does not implement the vld1q_u8_x4() intrinsic, so maybe we should foll Exactly my point: that's how it's done at https://github.com/DLTcollab/sse2neon/blob/471743745dd09798fb7df87a9b7f31f543aa31da/sse2neon.h#L303-L316 -- To view, visit http://gerrit.cloudera.org:8080/16907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2538e6ca321e89edb46d54f620b305b8e005f173 Gerrit-Change-Number: 16907 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jan 2021 06:35:15 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3225 Fixes error 'vld1q u8 x4' was not declared
huangtianhua...@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/16907 ) Change subject: KUDU-3225 Fixes error 'vld1q_u8_x4' was not declared .. Patch Set 1: But if you see the comment of https://github.com/DLTcollab/sse2neon/pull/31, you will find: "As I search the source code on the GCC github, it does not contain vld1q_u8_x4. The newest release version is GCC 9.3, hence the modification is acceptable. But it needs to be taken care with as the newer GCC is released." Now the lastest stable release of GCC is 10.2, so it doesn't work if we forceinline only for the versions <= 9.3 -- To view, visit http://gerrit.cloudera.org:8080/16907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2538e6ca321e89edb46d54f620b305b8e005f173 Gerrit-Change-Number: 16907 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jan 2021 06:45:05 + Gerrit-HasComments: No
[kudu-CR] KUDU-3225 Fixes error 'vld1q u8 x4' was not declared
huangtianhua...@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/16907 ) Change subject: KUDU-3225 Fixes error 'vld1q_u8_x4' was not declared .. Patch Set 1: Or maybe we only follow the community to change it if needed later? -- To view, visit http://gerrit.cloudera.org:8080/16907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2538e6ca321e89edb46d54f620b305b8e005f173 Gerrit-Change-Number: 16907 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jan 2021 06:46:52 + Gerrit-HasComments: No
[kudu-CR] (wip) KUDU-2612: restrict TxnStatusManager calls to be made by the leader only
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16648 to look at the new patch set (#3). Change subject: (wip) KUDU-2612: restrict TxnStatusManager calls to be made by the leader only .. (wip) KUDU-2612: restrict TxnStatusManager calls to be made by the leader only Currently, even though a non-leader TxnStatusManager will be unable to write to the underlying table (in the Raft subsystem the write would be aborted), we may want to restrict calls to be made by the leader TxnStatusManagers only. The motivation is to provide a more robust system, which avoids cases when the request was sent to a laggy follower, we may end up failing the request with an error. This patch introduces ScopedLeaderSharedLock (similar to the one in Catalog Manager) to be used to ensure the requests were sent to leaders only and to block all other operations while reloading the persistent transaction status metadata upon leadership changes. Note that during failover the leader replica will wait until in-flight ops in the previous consensus term to be applied before reloading the metadata. wip: txn_status_manager-itest is flaky. Add more comments. Change-Id: I42c1ad095dcb4bdffcbe0ecf9631a60bac208c2a --- M src/kudu/integration-tests/txn_status_manager-itest.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/txn_manager-test.cc M src/kudu/tablet/tablet_replica-test-base.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/transactions/txn_status_tablet.h M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 16 files changed, 763 insertions(+), 146 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/16648/3 -- To view, visit http://gerrit.cloudera.org:8080/16648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I42c1ad095dcb4bdffcbe0ecf9631a60bac208c2a Gerrit-Change-Number: 16648 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3225 Fixes error 'vld1q u8 x4' was not declared
Hello Quanlong Huang, Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16907 to look at the new patch set (#2). Change subject: KUDU-3225 Fixes error 'vld1q_u8_x4' was not declared .. KUDU-3225 Fixes error 'vld1q_u8_x4' was not declared GCC does not implement 'vld1q_u8_x4' instrinsic, to forceinline it for GCC versions smaller than or equal to 9.3 which as default version of Ubuntu 20.04. Change-Id: I2538e6ca321e89edb46d54f620b305b8e005f173 --- M src/kudu/util/sse2neon.h 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/16907/2 -- To view, visit http://gerrit.cloudera.org:8080/16907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2538e6ca321e89edb46d54f620b305b8e005f173 Gerrit-Change-Number: 16907 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong
[kudu-CR] (wip) KUDU-2612: restrict TxnStatusManager calls to be made by the leader only
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/16648 )
Change subject: (wip) KUDU-2612: restrict TxnStatusManager calls to be made by
the leader only
..
Patch Set 3:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/tablet/tablet_replica.cc@154
PS1, Line 154:*meta_->table_type() ==
TableTypePB::TXN_STATUS_TABLE ?
> Do we also need to call cb() here?
Yeah, good point, I don't see reasons not to, added it.
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/tablet/tablet_replica.cc@147
PS1, Line 147: cmeta_manager_(DCHECK_NOTNULL(std::move(cmeta_manager))),
: local_peer_pb_(std::move(local_peer_pb)),
: log_anchor_registry_(new LogAnchorRegistry()),
: apply_pool_(apply_pool),
:
reload_txn_status_tablet_pool_(reload_txn_status_tablet_pool),
: shutdown_latch_(shutdown_latch),
: txn_coordinator_(meta_->table_type() &&
:*meta_->table_type() ==
TableTypePB::TXN_STATUS_TABLE ?
:DCHECK_NOTNULL(txn_
> FWIW I think this would be more plumbing that it's worth.
Agree with Andrew on this. Alexey, let me know if you think otherwise
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/tablet/tablet_replica.cc@302
PS1, Line 302: PREFIX(WA
> nit: why does this need to be a lambda?
Just want to be clear this is for leadership change cb, but I guess it is a bit
redundant. Removed.
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/tablet/tablet_replica.cc@302
PS1, Line 302: LOG_WITH_PREFIX(WARNIN
> Just curious: is this going to be called if leadership status hasn't change
Right, this is going to be called for any leadership status change, even though
the former leader can wins the latest election again. As we discussed offline,
it is not clear what the performance implication will be, so adding a TODO in
case we think optimization of not reloading the metadata for this case.
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/transactions/txn_status_manager.h
File src/kudu/transactions/txn_status_manager.h:
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/transactions/txn_status_manager.h@95
PS1, Line 95: explicit ScopedLeaderSharedLock(TxnStatusManager* txn_status);
> warning: invalid case style for parameter 'txn_status_' [readability-identi
Done
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/transactions/txn_status_manager.cc
File src/kudu/transactions/txn_status_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/transactions/txn_status_manager.cc@62
PS1, Line 62:
DEFINE_int32(txn_status_manager_inject_latency_load_from_tablet_ms, 0,
> warning: using decl 'CoordinateTransactionResponsePB' is unused [misc-unuse
Done
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/transactions/txn_status_manager.cc@83
PS1, Line 83:
> nit: construction
Done
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/transactions/txn_status_manager.cc@189
PS1, Line 189:
> nit: remove extra slashes?
Done
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/transactions/txn_status_manager.cc@300
PS1, Line 300: if (consensus->role() != RaftPeerPB::LEADER) {
> warning: missing username/bug in TODO [google-readability-todo]
Done
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/transactions/txn_status_manager.cc@324
PS1, Line 324:
RETURN_NOT_OK(status_tablet_.tablet_replica_->op_tracker()->WaitForAllToFinish(timeout));
> warning: the parameter 'func' is copied for each invocation but only used a
Done
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/transactions/txn_status_manager.cc@347
PS1, Line 347: return Status::NotAuthoriz
> nit: you can use __builtin_unreachable()
Done
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/transactions/txn_status_manager.cc@357
PS1, Line 357: table
> nit: This gets used once -- why does it need to be a lambda?
Hmm, since 'check' expects 'std::function', what do you suggest
instead of using lambda?
http://gerrit.cloudera.org:8080/#/c/16648/2/src/kudu/transactions/txn_status_manager.cc
File src/kudu/transactions/txn_status_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16648/2/src/kudu/transactions/txn_status_manager.cc@202
PS2, Line 202: return;
> What happens if an instance of this lock is being constructed and the table
Right, updated the code to ensure the lock is only hold for tablet replica that
is fully started as Andrew suggested.
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16648/1/src/kudu/tserver/ts_tablet_manager.cc@386
PS1, Line 386: disks
[kudu-CR] KUDU-3225 Fixes error 'vld1q u8 x4' was not declared
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16907 ) Change subject: KUDU-3225 Fixes error 'vld1q_u8_x4' was not declared .. Patch Set 2: > Or maybe we only follow the community to change it if needed later? Yes, I think it's easier to keep it in sync with the upstream version as much as possible unless there is a particular reason to do otherwise. -- To view, visit http://gerrit.cloudera.org:8080/16907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2538e6ca321e89edb46d54f620b305b8e005f173 Gerrit-Change-Number: 16907 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jan 2021 07:18:03 + Gerrit-HasComments: No
[kudu-CR] KUDU-3225 Fixes error 'vld1q u8 x4' was not declared
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16907 ) Change subject: KUDU-3225 Fixes error 'vld1q_u8_x4' was not declared .. Patch Set 2: Code-Review+1 (1 comment) Looks good to me, just a nit to update the commit description. http://gerrit.cloudera.org:8080/#/c/16907/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16907/2//COMMIT_MSG@10 PS2, Line 10: for GCC versions smaller than or equal to 9.3 nit: it seems this need to be updated to correspond to the newer code in PS2 -- To view, visit http://gerrit.cloudera.org:8080/16907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2538e6ca321e89edb46d54f620b305b8e005f173 Gerrit-Change-Number: 16907 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jan 2021 07:19:54 + Gerrit-HasComments: Yes
