[kudu-CR] [java] add method to deserialize scanner token into scanner builder
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16598 to look at the new patch set (#6). Change subject: [java] add method to deserialize scanner token into scanner builder .. [java] add method to deserialize scanner token into scanner builder To support build scanner with extra dynamic runtime filters. Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java 2 files changed, 48 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/16598/6 -- To view, visit http://gerrit.cloudera.org:8080/16598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 Gerrit-Change-Number: 16598 Gerrit-PatchSet: 6 Gerrit-Owner: Li Zhiming Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tserver] introduce filter type for ListTablets
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16560 ) Change subject: [tserver] introduce filter_type for ListTablets .. Patch Set 3: > We chatted about this a bit. Given the new lazy creation of the > transaction status table, seems this is no long required to unblock > tests. > > We may want this filtering in the future, given not all users may > want to see system tables. That said, it isn't obvious how > prominent this filtering should be (e.g. should we also filter the > web UI? should we update all tooling?). For now, let's let this sit > and circle back if any strong arguments for continuing this work > come up. SGTM -- To view, visit http://gerrit.cloudera.org:8080/16560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3f8d34a8c377fcdef0162eaf042498a6d41baa4a Gerrit-Change-Number: 16560 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 14 Oct 2020 23:09:59 + Gerrit-HasComments: No
[kudu-CR] [partitioning] KUDU-2671: Support for range specific HashSchemas.
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16596 )
Change subject: [partitioning] KUDU-2671: Support for range specific
HashSchemas.
..
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/16596/1/src/kudu/common/partition.h
File src/kudu/common/partition.h:
http://gerrit.cloudera.org:8080/#/c/16596/1/src/kudu/common/partition.h@149
PS1, Line 149: integer
> right now it's useless, but the idea was to replace the integer with the up
I guess it would be enough to have just the lower bound for a range given that
we don't allow ranges to intersect, right?
Also, why is it 'int', not 'std::string' as the type of the
Partition::partition_key_start_ member field would imply?
http://gerrit.cloudera.org:8080/#/c/16596/1/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/16596/1/src/kudu/common/partition.cc@366
PS1, Line 366: partitions
> nit: not your fault, but IMO it makes it more difficult to read to use 'par
+1
http://gerrit.cloudera.org:8080/#/c/16596/1/src/kudu/common/partition.cc@398
PS1, Line 398: RETURN_NOT_OK(EncodeRangeBounds(range_bounds, schema,
&bounds));
: RETURN_NOT_OK(EncodeRangeSplits(split_rows, schema,
&splits));
: RETURN_NOT_OK(SplitRangeBounds(schema, std::move(splits),
&bounds));
> If we don't have any 'split_rows', then the number of bounds would be known
style nit: it seems there three lines has shifted right; what might be the
reason?
http://gerrit.cloudera.org:8080/#/c/16596/1/src/kudu/common/partition.cc@410
PS1, Line 410: vector current_bound_hash_partitions =
vector(1);
nit: a shorter form might be
vector current_bound_hash_partitions(1);
http://gerrit.cloudera.org:8080/#/c/16596/1/src/kudu/common/partition.cc@669
PS1, Line 669: string PartitionSchema::HashSchemaPerPartitionDebugString(const
Partition& partition,
: const
vector& range_hash_schema,
: const Schema&
schema) const {
style nit: the parameters are not aligned properly
http://gerrit.cloudera.org:8080/#/c/16596/1/src/kudu/common/partition.cc@675
PS1, Line 675:
nit: reserving the space for 'components' might be a good idea; I guess here
it'd be
components.reserve(hash_bucket_schemas_.size() + 1);
http://gerrit.cloudera.org:8080/#/c/16596/1/src/kudu/common/partition.cc@686
PS1, Line 686: use default HashSchema when no range-specific hash partitions
are provided
style nit: in comments like this, in Kudu code we tend to use full sentences,
i.e. it should start with a capital letter and end with a period (dot).
http://gerrit.cloudera.org:8080/#/c/16596/1/src/kudu/common/partition.cc@695
PS1, Line 695: hash_buckets_[i]
How is it guaranteed that partition.hash_buckets_ has enough elements to have
index 'i' to be in the bounds of the container?
--
To view, visit http://gerrit.cloudera.org:8080/16596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8725f4bd072a81b05b36dfc7df0c074c172b4ce8
Gerrit-Change-Number: 16596
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Comment-Date: Wed, 14 Oct 2020 23:09:37 +
Gerrit-HasComments: Yes
[kudu-CR] [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16601 ) Change subject: [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig .. [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig I saw the AdminCliTest.TestUnsafeChangeConfigWithFiveReplicaConfig scenario failing pre-commit TSAN build [1] with the error like below: Timed out: Committed consensus opid_index does not equal 1 after waiting for 30.014s. Last opid: 2.2 As I understand, the reason for failing the assertion is an extra election round that has happened after startup of a tablet. This patch addresses the issue in the TestUnsafeChangeConfigWithFiveReplicaConfig and other scenarios which might be affected if same ever happens again. [1] http://jenkins.kudu.apache.org/job/kudu-gerrit/22585/BUILD_TYPE=TSAN Change-Id: I1e77c440b872a42c68b6d7d1cb0563fee0470fc2 Reviewed-on: http://gerrit.cloudera.org:8080/16601 Tested-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/tools/kudu-admin-test.cc 1 file changed, 17 insertions(+), 16 deletions(-) Approvals: Alexey Serbin: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1e77c440b872a42c68b6d7d1cb0563fee0470fc2 Gerrit-Change-Number: 16601 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612: add TxnManager::BeginTransaction()
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16586 ) Change subject: KUDU-2612: add TxnManager::BeginTransaction() .. KUDU-2612: add TxnManager::BeginTransaction() This patch adds the implementation of the BeginTransaction() method to the TxnManager along with tests to cover the newly introduced functionality. Change-Id: I51c476d92bb5b147ffd03fd9f3163ab86d581496 Reviewed-on: http://gerrit.cloudera.org:8080/16586 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M src/kudu/master/txn_manager-test.cc M src/kudu/master/txn_manager.cc M src/kudu/master/txn_manager.h M src/kudu/transactions/txn_system_client.cc M src/kudu/tserver/tablet_service.cc 5 files changed, 301 insertions(+), 23 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16586 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I51c476d92bb5b147ffd03fd9f3163ab86d581496 Gerrit-Change-Number: 16586 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612: add TxnManager::BeginTransaction()
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16586 ) Change subject: KUDU-2612: add TxnManager::BeginTransaction() .. Patch Set 3: (1 comment) Thank you for review! http://gerrit.cloudera.org:8080/#/c/16586/3/src/kudu/master/txn_manager.cc File src/kudu/master/txn_manager.cc: http://gerrit.cloudera.org:8080/#/c/16586/3/src/kudu/master/txn_manager.cc@184 PS3, Line 184: try_txn_id / txn_status_table_range_span_) * : txn_status_table_range_span_; : const auto hb = lb + txn_status_table_range_span_; > I see. Yep, makes sense we'd have to examine the highest existing range. I'm planning to address the TODO above in a follow-up patch. I think it's not a high priority item, but of course it introduced a bit of technical debt. I'm planning to get back to related items once the client-side API for transaction functionality is taken care of. -- To view, visit http://gerrit.cloudera.org:8080/16586 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51c476d92bb5b147ffd03fd9f3163ab86d581496 Gerrit-Change-Number: 16586 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Oct 2020 19:33:31 + Gerrit-HasComments: Yes
[kudu-CR] [java] Fixes for JDK 12 to 15
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16600 ) Change subject: [java] Fixes for JDK 12 to 15 .. [java] Fixes for JDK 12 to 15 This patch fixes various issues found when testing with JDK 12 to JDK 15. The main fix is changing `HostAndPort.toString()`. `InetSocketAddress.toString()` changed in JDK15 to include “unresolved” in the ouput. I also needed to fix a few SpotBugs issues and disabled `kudu-jepsen` on JDK 12+ given Clojure only supports JDK 8 and 11. The Hive tests do not run due to HIVE-21508. An upgrade to Hive 3.2.0 once available should resolve that issue. Change-Id: Ie992d089a06e8d4afecda39a234e19a45730381e Reviewed-on: http://gerrit.cloudera.org:8080/16600 Tested-by: Grant Henke Reviewed-by: Hao Hao --- M java/config/spotbugs/excludeFilter.xml M java/gradle/dependencies.gradle M java/gradle/tests.gradle M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-client/src/main/java/org/apache/kudu/client/HostAndPort.java M java/kudu-client/src/test/java/org/apache/kudu/util/TestNetUtil.java M java/kudu-jepsen/build.gradle M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala 10 files changed, 30 insertions(+), 9 deletions(-) Approvals: Grant Henke: Verified Hao Hao: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie992d089a06e8d4afecda39a234e19a45730381e Gerrit-Change-Number: 16600 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16601 ) Change subject: [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e77c440b872a42c68b6d7d1cb0563fee0470fc2 Gerrit-Change-Number: 16601 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Oct 2020 18:57:30 + Gerrit-HasComments: No
[kudu-CR] KUDU-2612: add TxnManager::BeginTransaction()
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16586 ) Change subject: KUDU-2612: add TxnManager::BeginTransaction() .. Patch Set 3: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/16586/3/src/kudu/master/txn_manager.cc File src/kudu/master/txn_manager.cc: http://gerrit.cloudera.org:8080/#/c/16586/3/src/kudu/master/txn_manager.cc@184 PS3, Line 184: try_txn_id / txn_status_table_range_span_) * : txn_status_table_range_span_; : const auto hb = lb + txn_status_table_range_span_; > If FLAGS_txn_manager_status_table_range_partition_span is changed, it's ne I see. Yep, makes sense we'd have to examine the highest existing range. -- To view, visit http://gerrit.cloudera.org:8080/16586 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51c476d92bb5b147ffd03fd9f3163ab86d581496 Gerrit-Change-Number: 16586 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Oct 2020 18:52:00 + Gerrit-HasComments: Yes
[kudu-CR] [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig
Alexey Serbin has removed a vote on this change. Change subject: [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I1e77c440b872a42c68b6d7d1cb0563fee0470fc2 Gerrit-Change-Number: 16601 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] Fixes for JDK 12 to 15
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16600 ) Change subject: [java] Fixes for JDK 12 to 15 .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie992d089a06e8d4afecda39a234e19a45730381e Gerrit-Change-Number: 16600 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Oct 2020 18:45:22 + Gerrit-HasComments: No
[kudu-CR] [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16601 ) Change subject: [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e77c440b872a42c68b6d7d1cb0563fee0470fc2 Gerrit-Change-Number: 16601 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Oct 2020 18:45:47 + Gerrit-HasComments: No
[kudu-CR] [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16601 ) Change subject: [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig .. Patch Set 1: unrelated test failure in ToolTest.TestHmsList -- To view, visit http://gerrit.cloudera.org:8080/16601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e77c440b872a42c68b6d7d1cb0563fee0470fc2 Gerrit-Change-Number: 16601 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Oct 2020 18:46:02 + Gerrit-HasComments: No
[kudu-CR] KUDU-2612: add TxnManager::BeginTransaction()
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16586 ) Change subject: KUDU-2612: add TxnManager::BeginTransaction() .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/16586/3/src/kudu/master/txn_manager.cc File src/kudu/master/txn_manager.cc: http://gerrit.cloudera.org:8080/#/c/16586/3/src/kudu/master/txn_manager.cc@184 PS3, Line 184: try_txn_id / txn_status_table_range_span_) * : txn_status_table_range_span_; : const auto hb = lb + txn_status_table_range_span_; > I'm not sure I see how this works if txn_status_table_range_span_ is differ If FLAGS_txn_manager_status_table_range_partition_span is changed, it's necessary to drop existing range partitions in the transaction status table. Otherwise, it will brake here unless the TODO above is addressed. -- To view, visit http://gerrit.cloudera.org:8080/16586 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51c476d92bb5b147ffd03fd9f3163ab86d581496 Gerrit-Change-Number: 16586 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Oct 2020 18:42:42 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: initial implementation of TxnManager
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16527 ) Change subject: KUDU-2612: initial implementation of TxnManager .. KUDU-2612: initial implementation of TxnManager This is a first implementation of the TxnManager. The TxnManager class encapsulates the logic used by the TxnManagerService while serving RPC requests (see txn_manager.proto for the protobuf interface). The most essential piece of the logic to be implemented by this class is the assignment of an identifier for a new transaction and initialization of the transaction status table, along with creating of its new partitions, when needed. All other methods simply do proxying of corresponding requests to the underlying instance of TxnSystemClient. This changelist also contains test scenarios to cover the newly introduced functionality. The implementation of TxnManager::BeginTransaction() is moved into a follow-up changelist by request for simplify the process of reviewing these changes. TxnManager::KeepTransactionAlive() will be implemented as soon as the corresponding functionality is ready in the TxnStatusManager. Change-Id: Ie952977a3ae5f625d1283389f0be8afb79df7d8c Reviewed-on: http://gerrit.cloudera.org:8080/16527 Tested-by: Alexey Serbin Reviewed-by: Andrew Wong Reviewed-by: Hao Hao --- M src/kudu/master/CMakeLists.txt M src/kudu/master/master.cc M src/kudu/master/master.h A src/kudu/master/txn_manager-test.cc A src/kudu/master/txn_manager.cc A src/kudu/master/txn_manager.h A src/kudu/master/txn_manager.proto A src/kudu/master/txn_manager_service.cc A src/kudu/master/txn_manager_service.h M src/kudu/transactions/txn_status_manager.h M src/kudu/transactions/txn_system_client.cc 11 files changed, 1,203 insertions(+), 15 deletions(-) Approvals: Alexey Serbin: Verified Andrew Wong: Looks good to me, approved Hao Hao: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16527 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie952977a3ae5f625d1283389f0be8afb79df7d8c Gerrit-Change-Number: 16527 Gerrit-PatchSet: 8 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612: initial implementation of TxnManager
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16527 ) Change subject: KUDU-2612: initial implementation of TxnManager .. Patch Set 7: Code-Review+2 (6 comments) http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/master.cc File src/kudu/master/master.cc: http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/master.cc@103 PS5, Line 103: ecurity::TokenSigner; > I guess for real world scenarios in clusters which use transactions we shou Ack http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/master.cc@319 PS5, Line 319: //wake up a long-awaiting task and retry initialization : //right away when TxnManager receives a call. : static const MonoDelta kRetryInterval = MonoDelta::FromSeconds(1); : KLOG_EVERY_N_SECS(WARNING > TxnManager cannot complete initialization without tablet servers which host Ack http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.h File src/kudu/master/txn_manager.h: http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.h@102 PS5, Line 102: // Whether or not this instance is lazily initializ > There is nothing specific about this, it's just a random number so far. I Ack http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.cc File src/kudu/master/txn_manager.cc: http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.cc@49 PS5, Line 49: For Kudu clusters which use transactions we should prefer non-laz > It happens in case of RPC-level errors. Ack http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.cc@127 PS5, Line 127: const string& username, : co > The header has similar comment as well. I put this comment here as well fo Ack http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.proto File src/kudu/master/txn_manager.proto: http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.proto@94 PS5, Line 94: ime > I replaced the whole sentence with 'The transaction state at the time of pr Ack -- To view, visit http://gerrit.cloudera.org:8080/16527 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie952977a3ae5f625d1283389f0be8afb79df7d8c Gerrit-Change-Number: 16527 Gerrit-PatchSet: 7 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 14 Oct 2020 18:13:20 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3149: don't block op registration on MM mutex
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16580 ) Change subject: KUDU-3149: don't block op registration on MM mutex .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/16580/5/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: http://gerrit.cloudera.org:8080/#/c/16580/5/src/kudu/util/maintenance_manager.cc@236 PS5, Line 236: ops_to_register = std::move(ops_pending_registration_); > warning: Moved-from object 'ops_pending_registration_' of type 'std::map' i Done -- To view, visit http://gerrit.cloudera.org:8080/16580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a1b810f5b7ff6a22acc9b10b79ddffa8085c990 Gerrit-Change-Number: 16580 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 14 Oct 2020 18:11:04 + Gerrit-HasComments: Yes
[kudu-CR] [tserver] introduce filter type for ListTablets
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16560 ) Change subject: [tserver] introduce filter_type for ListTablets .. Patch Set 3: We chatted about this a bit. Given the new lazy creation of the transaction status table, seems this is no long required to unblock tests. We may want this filtering in the future, given not all users may want to see system tables. That said, it isn't obvious how prominent this filtering should be (e.g. should we also filter the web UI? should we update all tooling?). For now, let's let this sit and circle back if any strong arguments for continuing this work come up. -- To view, visit http://gerrit.cloudera.org:8080/16560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3f8d34a8c377fcdef0162eaf042498a6d41baa4a Gerrit-Change-Number: 16560 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 14 Oct 2020 18:10:50 + Gerrit-HasComments: No
[kudu-CR] KUDU-3149: don't block op registration on MM mutex
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16580
to look at the new patch set (#7).
Change subject: KUDU-3149: don't block op registration on MM mutex
..
KUDU-3149: don't block op registration on MM mutex
The maintenance manager's 'lock_' is a mutex that is taken upon access
to 'ops_', notably when iterating through 'ops_' to find the best op to
run. This particular critical section can last quite a while, as finding
the best op entails computing stats for each op, which is expensive for
compactions, blocking op registration and thus tablet bootstrapping.
This patch addresses the issue by buffering calls to RegisterOp() into a
separate op map protected by a separate spinlock, and periodically
merging the separate map into 'ops_'.
I added a unit test for the maintenance manager change, and added a
single-node test to contend several tablets' bootstrap with compactions.
I ran this with and without op buffering (via a flag that I have removed
from this patch) with the following results:
[awong@va1022 release]$ for i in {1..5}; do ./bin/tablet_server-test
--gtest_filter=*WithConcurrent* --num_tablets=300 --num_rowsets_per_tablet=30
--buffer_op_registration=true |& grep "waiting for all bootstraps to finish";
done
I1014 02:19:25.452993 80415 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.787suser 0.000s sys 0.002s
I1014 02:19:43.039741 82020 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.641suser 0.000s sys 0.004s
I1014 02:20:02.907203 83608 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.769suser 0.001s sys 0.002s
I1014 02:20:21.779570 85260 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.758suser 0.002s sys 0.001s
I1014 02:20:40.687155 86874 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.682suser 0.001s sys 0.002s
Average real time with op buffering: 0.727s
[awong@va1022 release]$ for i in {1..5}; do ./bin/tablet_server-test
--gtest_filter=*WithConcurrent* --num_tablets=300 --num_rowsets_per_tablet=30
--buffer_op_registration=false |& grep "waiting for all bootstraps to finish";
done
I1014 02:21:13.89 88559 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.538suser 0.002s sys 0.001s
I1014 02:21:33.119479 90172 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.316suser 0.001s sys 0.002s
I1014 02:21:53.929062 91816 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.393suser 0.003s sys 0.001s
I1014 02:22:12.764689 93439 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.356suser 0.003s sys 0.000s
I1014 02:22:31.138669 95042 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.516suser 0.000s sys 0.003s
Average real time without op buffering: 1.424s
Change-Id: I4a1b810f5b7ff6a22acc9b10b79ddffa8085c990
---
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/maintenance_manager.h
4 files changed, 164 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/16580/7
--
To view, visit http://gerrit.cloudera.org:8080/16580
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a1b810f5b7ff6a22acc9b10b79ddffa8085c990
Gerrit-Change-Number: 16580
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16601 Change subject: [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig .. [kudu-admin-test] fix TestUnsafeChangeConfigWithFiveReplicaConfig I saw the AdminCliTest.TestUnsafeChangeConfigWithFiveReplicaConfig scenario failing pre-commit TSAN build [1] with the error like below: Timed out: Committed consensus opid_index does not equal 1 after waiting for 30.014s. Last opid: 2.2 As I understand, the reason for failing the assertion is an extra election round that has happened after startup of a tablet. This patch addresses the issue in the TestUnsafeChangeConfigWithFiveReplicaConfig and other scenarios which might be affected if same ever happens again. [1] http://jenkins.kudu.apache.org/job/kudu-gerrit/22585/BUILD_TYPE=TSAN Change-Id: I1e77c440b872a42c68b6d7d1cb0563fee0470fc2 --- M src/kudu/tools/kudu-admin-test.cc 1 file changed, 17 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/16601/1 -- To view, visit http://gerrit.cloudera.org:8080/16601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1e77c440b872a42c68b6d7d1cb0563fee0470fc2 Gerrit-Change-Number: 16601 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-2612: add TxnManager::BeginTransaction()
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16586 ) Change subject: KUDU-2612: add TxnManager::BeginTransaction() .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/16586/3/src/kudu/master/txn_manager.cc File src/kudu/master/txn_manager.cc: http://gerrit.cloudera.org:8080/#/c/16586/3/src/kudu/master/txn_manager.cc@184 PS3, Line 184: try_txn_id / txn_status_table_range_span_) * : txn_status_table_range_span_; : const auto hb = lb + txn_status_table_range_span_; I'm not sure I see how this works if txn_status_table_range_span_ is different from FLAGS_txn_manager_status_table_range_partition_span. It's not listed as runtime, but isn't it possible that it's changed? -- To view, visit http://gerrit.cloudera.org:8080/16586 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51c476d92bb5b147ffd03fd9f3163ab86d581496 Gerrit-Change-Number: 16586 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Oct 2020 17:58:50 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: initial implementation of TxnManager
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16527 ) Change subject: KUDU-2612: initial implementation of TxnManager .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16527 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie952977a3ae5f625d1283389f0be8afb79df7d8c Gerrit-Change-Number: 16527 Gerrit-PatchSet: 7 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 14 Oct 2020 17:50:42 + Gerrit-HasComments: No
[kudu-CR] KUDU-3149: don't block op registration on MM mutex
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16580
to look at the new patch set (#6).
Change subject: KUDU-3149: don't block op registration on MM mutex
..
KUDU-3149: don't block op registration on MM mutex
The maintenance manager's 'lock_' is a mutex that is taken upon access
to 'ops_', notably when iterating through 'ops_' to find the best op to
run. This particular critical section can last quite a while, as finding
the best op entails computing stats for each op, which is expensive for
compactions, blocking op registration and thus tablet bootstrapping.
This patch addresses the issue by buffering calls to RegisterOp() into a
separate op map protected by a separate spinlock, and periodically
merging the separate map into 'ops_'.
I added a unit test for the maintenance manager change, and added a
single-node test to contend several tablets' bootstrap with compactions.
I ran this with and without op buffering (via a flag that I have removed
from this patch) with the following results:
[awong@va1022 release]$ for i in {1..5}; do ./bin/tablet_server-test
--gtest_filter=*WithConcurrent* --num_tablets=300 --num_rowsets_per_tablet=30
--buffer_op_registration=true |& grep "waiting for all bootstraps to finish";
done
I1014 02:19:25.452993 80415 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.787suser 0.000s sys 0.002s
I1014 02:19:43.039741 82020 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.641suser 0.000s sys 0.004s
I1014 02:20:02.907203 83608 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.769suser 0.001s sys 0.002s
I1014 02:20:21.779570 85260 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.758suser 0.002s sys 0.001s
I1014 02:20:40.687155 86874 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.682suser 0.001s sys 0.002s
Average real time with op buffering: 0.727s
[awong@va1022 release]$ for i in {1..5}; do ./bin/tablet_server-test
--gtest_filter=*WithConcurrent* --num_tablets=300 --num_rowsets_per_tablet=30
--buffer_op_registration=false |& grep "waiting for all bootstraps to finish";
done
I1014 02:21:13.89 88559 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.538suser 0.002s sys 0.001s
I1014 02:21:33.119479 90172 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.316suser 0.001s sys 0.002s
I1014 02:21:53.929062 91816 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.393suser 0.003s sys 0.001s
I1014 02:22:12.764689 93439 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.356suser 0.003s sys 0.000s
I1014 02:22:31.138669 95042 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.516suser 0.000s sys 0.003s
Average real time without op buffering: 1.424s
Change-Id: I4a1b810f5b7ff6a22acc9b10b79ddffa8085c990
---
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/maintenance_manager.h
4 files changed, 163 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/16580/6
--
To view, visit http://gerrit.cloudera.org:8080/16580
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a1b810f5b7ff6a22acc9b10b79ddffa8085c990
Gerrit-Change-Number: 16580
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3149: don't block op registration on MM mutex
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/16580 )
Change subject: KUDU-3149: don't block op registration on MM mutex
..
Patch Set 5:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/16580/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16580/4//COMMIT_MSG@18
PS4, Line 18:
> nit: This patch ?
Done
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.h@371
PS4, Line 371: void Me
> nit: is 'mutable' really needed here?
Done
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc@a262
PS4, Line 262:
:
:
> Just curious: is this a critical invariant to maintain? Was it rather a DC
I don't think it's critical given it's unregistered either way.
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc@231
PS4, Line 231: void MaintenanceManager::MergePendingOpRegistrationsUnlocked() {
> Given this doesn't actually register the op, maybe we should rename the met
I updated this to try registering the op if it can.
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc@236
PS4, Line 236: ops_to_register = std::move
> nit: could use 'auto' here
Done
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc@237
PS4, Line 237:
> I guess this construction of value_type object might be dropped once using
Done
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc@237
PS4, Line 237:
> nit: maybe, using EmplaceOrDie would be more idiomatic here?
Done
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc@259
PS4, Line 259: }
: }
> nit: to prevent double lookup, it might be rewritten as
Done
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc@263
PS4, Line 263: {
: std::lock_guard guard(lock_);
> nit: to prevent double lookup, I guess it's possible to compare the return
Done
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc@291
PS4, Line 291: bool MaintenanceManager::disabled_for_tests() const {
> Nit, this could be encapsulated in a method
Done
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc@302
PS4, Line 302:
> Right, the 'emplace' method signature assumes just a single element to add
Yea, emplace() doesn't work with these iterator arguments.
http://gerrit.cloudera.org:8080/#/c/16580/4/src/kudu/util/maintenance_manager.cc@566
PS4, Line 566: op->name());
> Should method be updated to accommodate the introduction of the ops_pending
Done
--
To view, visit http://gerrit.cloudera.org:8080/16580
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a1b810f5b7ff6a22acc9b10b79ddffa8085c990
Gerrit-Change-Number: 16580
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 14 Oct 2020 17:32:26 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3149: don't block op registration on MM mutex
Hello Alexey Serbin, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16580
to look at the new patch set (#5).
Change subject: KUDU-3149: don't block op registration on MM mutex
..
KUDU-3149: don't block op registration on MM mutex
The maintenance manager's 'lock_' is a mutex that is taken upon access
to 'ops_', notably when iterating through 'ops_' to find the best op to
run. This particular critical section can last quite a while, as finding
the best op entails computing stats for each op, which is expensive for
compactions, blocking op registration and thus tablet bootstrapping.
This patch addresses the issue by buffering calls to RegisterOp() into a
separate op map protected by a separate spinlock, and periodically
merging the separate map into 'ops_'.
I added a unit test for the maintenance manager change, and added a
single-node test to contend several tablets' bootstrap with compactions.
I ran this with and without op buffering (via a flag that I have removed
from this patch) with the following results:
[awong@va1022 release]$ for i in {1..5}; do ./bin/tablet_server-test
--gtest_filter=*WithConcurrent* --num_tablets=300 --num_rowsets_per_tablet=30
--buffer_op_registration=true |& grep "waiting for all bootstraps to finish";
done
I1014 02:19:25.452993 80415 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.787suser 0.000s sys 0.002s
I1014 02:19:43.039741 82020 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.641suser 0.000s sys 0.004s
I1014 02:20:02.907203 83608 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.769suser 0.001s sys 0.002s
I1014 02:20:21.779570 85260 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.758suser 0.002s sys 0.001s
I1014 02:20:40.687155 86874 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 0.682suser 0.001s sys 0.002s
Average real time with op buffering: 0.727s
[awong@va1022 release]$ for i in {1..5}; do ./bin/tablet_server-test
--gtest_filter=*WithConcurrent* --num_tablets=300 --num_rowsets_per_tablet=30
--buffer_op_registration=false |& grep "waiting for all bootstraps to finish";
done
I1014 02:21:13.89 88559 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.538suser 0.002s sys 0.001s
I1014 02:21:33.119479 90172 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.316suser 0.001s sys 0.002s
I1014 02:21:53.929062 91816 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.393suser 0.003s sys 0.001s
I1014 02:22:12.764689 93439 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.356suser 0.003s sys 0.000s
I1014 02:22:31.138669 95042 tablet_server-test.cc:4405] Time spent waiting for
all bootstraps to finish: real 1.516suser 0.000s sys 0.003s
Average real time without op buffering: 1.424s
Change-Id: I4a1b810f5b7ff6a22acc9b10b79ddffa8085c990
---
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/maintenance_manager.h
4 files changed, 163 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/16580/5
--
To view, visit http://gerrit.cloudera.org:8080/16580
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a1b810f5b7ff6a22acc9b10b79ddffa8085c990
Gerrit-Change-Number: 16580
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] Fixes for JDK 12 to 15
Grant Henke has removed a vote on this change. Change subject: [java] Fixes for JDK 12 to 15 .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ie992d089a06e8d4afecda39a234e19a45730381e Gerrit-Change-Number: 16600 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] Fixes for JDK 12 to 15
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16600 ) Change subject: [java] Fixes for JDK 12 to 15 .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie992d089a06e8d4afecda39a234e19a45730381e Gerrit-Change-Number: 16600 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Oct 2020 16:41:19 + Gerrit-HasComments: No
[kudu-CR] [java] add method to deserialize scanner token into scanner builder
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16598 ) Change subject: [java] add method to deserialize scanner token into scanner builder .. Patch Set 5: Thanks for the patch! I know the change/functionality is trivial, but do you mind adding a test that deserializes into a scan token builder, adds a dynamic filter, and uses the token? That way we ensure we don't break the use case in the future. -- To view, visit http://gerrit.cloudera.org:8080/16598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 Gerrit-Change-Number: 16598 Gerrit-PatchSet: 5 Gerrit-Owner: Li Zhiming Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Oct 2020 16:01:26 + Gerrit-HasComments: No
[kudu-CR] [java] Fixes for JDK 12 to 15
Hello Attila Bukor, Kudu Jenkins, Hao Hao, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16600 to look at the new patch set (#2). Change subject: [java] Fixes for JDK 12 to 15 .. [java] Fixes for JDK 12 to 15 This patch fixes various issues found when testing with JDK 12 to JDK 15. The main fix is changing `HostAndPort.toString()`. `InetSocketAddress.toString()` changed in JDK15 to include “unresolved” in the ouput. I also needed to fix a few SpotBugs issues and disabled `kudu-jepsen` on JDK 12+ given Clojure only supports JDK 8 and 11. The Hive tests do not run due to HIVE-21508. An upgrade to Hive 3.2.0 once available should resolve that issue. Change-Id: Ie992d089a06e8d4afecda39a234e19a45730381e --- M java/config/spotbugs/excludeFilter.xml M java/gradle/dependencies.gradle M java/gradle/tests.gradle M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-client/src/main/java/org/apache/kudu/client/HostAndPort.java M java/kudu-client/src/test/java/org/apache/kudu/util/TestNetUtil.java M java/kudu-jepsen/build.gradle M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala 10 files changed, 30 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/16600/2 -- To view, visit http://gerrit.cloudera.org:8080/16600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie992d089a06e8d4afecda39a234e19a45730381e Gerrit-Change-Number: 16600 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] Fixes for JDK 12 to 15
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16600 Change subject: [java] Fixes for JDK 12 to 15 .. [java] Fixes for JDK 12 to 15 This patch fixes various issues found when testing with JDK 12 to JDK 15. The changes required are: The main fix is chaning `HostAndPort.toString()`. InetSocketAddress.toString() changed in JDK15 to include “unresolved” in the ouput. I also needed to fix a few SpotBugs issues and disabled `kudu-jepsen` on JDK 12+ given Clojure only supports JDK 8 and 11. The Hive tests do not run due to HIVE-21508. An upgrade to Hive 3.2.0 once available should resolve the issue. Change-Id: Ie992d089a06e8d4afecda39a234e19a45730381e --- M java/config/spotbugs/excludeFilter.xml M java/gradle/dependencies.gradle M java/gradle/tests.gradle M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-client/src/main/java/org/apache/kudu/client/HostAndPort.java M java/kudu-client/src/test/java/org/apache/kudu/util/TestNetUtil.java M java/kudu-jepsen/build.gradle M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala 10 files changed, 30 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/16600/1 -- To view, visit http://gerrit.cloudera.org:8080/16600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie992d089a06e8d4afecda39a234e19a45730381e Gerrit-Change-Number: 16600 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [java] add method to deserialize scanner token into scanner builder
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16598 to look at the new patch set (#5). Change subject: [java] add method to deserialize scanner token into scanner builder .. [java] add method to deserialize scanner token into scanner builder To support build scanner with extra dynamic runtime filters. Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java 1 file changed, 17 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/16598/5 -- To view, visit http://gerrit.cloudera.org:8080/16598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 Gerrit-Change-Number: 16598 Gerrit-PatchSet: 5 Gerrit-Owner: Li Zhiming Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] add method to deserialize scanner token into scanner builder
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16598 to look at the new patch set (#4). Change subject: [java] add method to deserialize scanner token into scanner builder .. [java] add method to deserialize scanner token into scanner builder To support build scanner with extra dynamic runtime filters. Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java 1 file changed, 17 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/16598/4 -- To view, visit http://gerrit.cloudera.org:8080/16598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 Gerrit-Change-Number: 16598 Gerrit-PatchSet: 4 Gerrit-Owner: Li Zhiming Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] add method to deserialize scanner token into scanner builder
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16598 to look at the new patch set (#3). Change subject: [java] add method to deserialize scanner token into scanner builder .. [java] add method to deserialize scanner token into scanner builder To support build scanner with extra dynamic runtime filters. Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java 1 file changed, 17 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/16598/3 -- To view, visit http://gerrit.cloudera.org:8080/16598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 Gerrit-Change-Number: 16598 Gerrit-PatchSet: 3 Gerrit-Owner: Li Zhiming Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] add method to deserialize scanner token into scanner builder
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16598 to look at the new patch set (#2). Change subject: [java] add method to deserialize scanner token into scanner builder .. [java] add method to deserialize scanner token into scanner builder To support build scanner with extra dynamic runtime filters. Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java 1 file changed, 17 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/16598/2 -- To view, visit http://gerrit.cloudera.org:8080/16598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 Gerrit-Change-Number: 16598 Gerrit-PatchSet: 2 Gerrit-Owner: Li Zhiming Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] add method to deserialize scanner token into scanner builder
Li Zhiming has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16598 Change subject: [java] add method to deserialize scanner token into scanner builder .. [java] add method to deserialize scanner token into scanner builder To support build scanner with extra dynamic runtime filters. Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java 1 file changed, 15 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/16598/1 -- To view, visit http://gerrit.cloudera.org:8080/16598 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I17b24a8e9dec846583ad0940de19b96b672bd603 Gerrit-Change-Number: 16598 Gerrit-PatchSet: 1 Gerrit-Owner: Li Zhiming
