[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16495 to look at the new patch set (#2). Change subject: WIP KUDU-2612 p2 (b): add transaction status retrieval .. WIP KUDU-2612 p2 (b): add transaction status retrieval After offline discussions with Andrew, it became clear that TxnManager should provide an asynchronous interface to commit a transaction, i.e. something similar to CreateTable()/IsCreateTableDone(). To implement that, the TxnManager needs to check for the status of the transaction after initiating the commit phase by issuing corresponding call to TxnStatusManager (that's implemented as CoordinateTransaction() RPC to TabletServerAdminService with BEGIN_COMMIT_TXN operation type). This patch introduces the required server-side piece to retrieve the information on a transaction status from the TxnStatusManager. I'm planning to introduce corresponding bindings via the TxnSystemClient in a separate changelist. This is a follow-up to efd8c4f165460b7fa337b8ebd1856b10bc274311. WIP: * I'd like to get some feedback on this approach: -- Is it OK to have a method to fetch transaction status as another operation type for the TxnStatusManager? -- Is it OK to use PB structures for the status of a transaction or we want to wrap that into something else? -- Anything else? Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 --- M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/CMakeLists.txt 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/CMakeLists.txt M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver_admin.proto 9 files changed, 165 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/16495/2 -- To view, visit http://gerrit.cloudera.org:8080/16495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 Gerrit-Change-Number: 16495 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP [security] report error details for Kerberos init failure
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16496 Change subject: WIP [security] report error details for Kerberos init failure .. WIP [security] report error details for Kerberos init failure WIP: * add functionality to add includedir into the krb5.conf * cleanup the code Change-Id: Ic4eca298b9796b311010f81d04eea8507a9c56c1 --- M src/kudu/security/init.cc M src/kudu/security/test/mini_kdc-test.cc M src/kudu/security/test/mini_kdc.cc 3 files changed, 59 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/16496/1 -- To view, visit http://gerrit.cloudera.org:8080/16496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic4eca298b9796b311010f81d04eea8507a9c56c1 Gerrit-Change-Number: 16496 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-3192: [client] Expose the cluster ID in the client KuduTable
Grant Henke has removed a vote on this change. Change subject: KUDU-3192: [client] Expose the cluster ID in the client KuduTable .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ia1f5a451aaa44834534d2387ee1c9aa9cf95dd37 Gerrit-Change-Number: 16493 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3192: [hms] Leverage the cluster ID in HMS sync
Grant Henke has removed a vote on this change. Change subject: KUDU-3192: [hms] Leverage the cluster ID in HMS sync .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16494 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I865b418a3cc4e11c889cc4757cd940831c43af17 Gerrit-Change-Number: 16494 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3192: [client] Expose the cluster ID in the client KuduTable
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16493 ) Change subject: KUDU-3192: [client] Expose the cluster ID in the client KuduTable .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia1f5a451aaa44834534d2387ee1c9aa9cf95dd37 Gerrit-Change-Number: 16493 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 13:15:20 + Gerrit-HasComments: No
[kudu-CR] KUDU-3192: [hms] Leverage the cluster ID in HMS sync
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16494 ) Change subject: KUDU-3192: [hms] Leverage the cluster ID in HMS sync .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16494 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I865b418a3cc4e11c889cc4757cd940831c43af17 Gerrit-Change-Number: 16494 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 23 Sep 2020 13:15:56 + Gerrit-HasComments: No
[kudu-CR](branch-1.12.x) [kudu-tool-test] fix ClusterNameResolverFileCorrupt with glibc 2.31
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16491 ) Change subject: [kudu-tool-test] fix ClusterNameResolverFileCorrupt with glibc 2.31 .. [kudu-tool-test] fix ClusterNameResolverFileCorrupt with glibc 2.31 This patch updates the expected error message in the ToolTest.ClusterNameResolverFileCorrupt scenario to make the test pass on Ubuntu 20.04 LTS (it should fix it on other platforms which use newer glibc versions). Without this patch, the scenario failed on Ubuntu 20.04 LTS: src/kudu/tools/kudu-tool-test.cc:637: Failure Value of: stderr Expected: has substring "Network error: Could not connect to the cluster: unable to resolve address for bad: Name or service not known" Actual: "W0919 23:39:50.032436 1041101 flags.cc:405] Enabled unsafe flag: --openssl_security_level_override=0\nW0919 23:39:50.032559 1041101 flags.cc:405] Enabled unsafe flag: --never_fsync=true\nNetwork error: Could not connect to the cluster: unable to resolve address for bad: Temporary failure in name resolution" Instead of tailoring the errno-converted message for every platform, it's easier to rely in the essential part of it that corresponds to the DNS resolver failure. In addition, I did a minor cleanup on the code around, removing calls of the Substitute() function where a fixed string is enough. Change-Id: I3eb0991cb2d4311051e55e231cb4fe6d065aa632 Reviewed-on: http://gerrit.cloudera.org:8080/16478 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke (cherry picked from commit d8ab44c6864172d4b990adf31f7d84fd33e4ae15) Reviewed-on: http://gerrit.cloudera.org:8080/16491 --- M src/kudu/tools/kudu-tool-test.cc 1 file changed, 4 insertions(+), 9 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: merged Gerrit-Change-Id: I3eb0991cb2d4311051e55e231cb4fe6d065aa632 Gerrit-Change-Number: 16491 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR](branch-1.12.x) [thirdparty] fix LLVM compilation on Ubuntu 20.04
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16490 ) Change subject: [thirdparty] fix LLVM compilation on Ubuntu 20.04 .. [thirdparty] fix LLVM compilation on Ubuntu 20.04 This changelist adds a patch from the llvm-toolchain-9-9.0.1 Debian source package to fix compilation of LLVM 9.0 in Kudu thirdparty on Ubuntu 20.04 LTS. See [1], [2] for the original source of the patch. The patch is already included in the upstream LLVM repo and included into 10.0.0 and later releases. [1] https://reviews.llvm.org/rG947f9692440 [2] https://github.com/llvm/llvm-project/commit/947f9692440836dcb8d88b74b69dd379d85974ce Change-Id: Ifba0bf6ba660a536ed54ddce228a38470dd1c650 Reviewed-on: http://gerrit.cloudera.org:8080/16477 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke (cherry picked from commit 2bed4068c53fcc6084e8e8742437846cf417a746) Reviewed-on: http://gerrit.cloudera.org:8080/16490 --- M thirdparty/download-thirdparty.sh A thirdparty/patches/llvm-947f9692440836dcb8d88b74b69dd379d85974ce.patch 2 files changed, 84 insertions(+), 2 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: merged Gerrit-Change-Id: Ifba0bf6ba660a536ed54ddce228a38470dd1c650 Gerrit-Change-Number: 16490 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16321 to look at the new patch set (#6). Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master .. [master] KUDU-2181 Raft ChangeConfig request to add a master This change: - Adds hidden feature flag "--master_support_change_config" off by default. - RPC changes to add a master that initiates Raft config change and responds asynchronously. - RPC changes to report back member type(VOTER/NON_VOTER) of masters - Removes the cached master_addresses in catalog manager as it's no longer static. - Updates and adds comments in MasterOptions such that it's used to fetch master addresses only during master init time as masters can be added/removed dynamically with this change. - Updates ListMasters() to look at local Raft config instead of MasterOptions as the masters can be added/removed dynamically. If the new master can be caught up from the WAL then the master gets promoted to VOTER else it remains as NON_VOTER without master tablet copying support. Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c --- M src/kudu/common/wire_protocol.proto M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h A src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/master/master.proto M src/kudu/master/master_options.cc M src/kudu/master/master_options.h M src/kudu/master/master_path_handlers.cc M src/kudu/master/master_service.cc M src/kudu/master/master_service.h M src/kudu/master/mini_master.cc M src/kudu/master/sys_catalog.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/mini-cluster/external_mini_cluster.h M src/kudu/tools/tool_action_master.cc 20 files changed, 965 insertions(+), 55 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/16321/6 -- To view, visit http://gerrit.cloudera.org:8080/16321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c Gerrit-Change-Number: 16321 Gerrit-PatchSet: 6 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Bankim Bhavsar has posted comments on this change. (
http://gerrit.cloudera.org:8080/16321 )
Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master
..
Patch Set 5:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/catalog_manager.h@810
PS5, Line 810: kRemoveMaster
> +1
Remove master is not implemented. Removing the enum.
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/catalog_manager.cc@1419
PS5, Line 1419:
> nit: 4 spaces here
Done
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc
File src/kudu/master/dynamic_multi_master-test.cc:
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@126
PS5, Line 126: resp
> Should we also check resp.error() here? Same elsewhere.
Done
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@149
PS5, Line 149: // Start with an existing master daemon.
> nit: maybe, "Start with an existing master daemon's options, but modify the
Done
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@186
PS5, Line 186: ASSERT_EVENTUALLY([&] {
> I guess the idea was to protect against fluctuations of master leadership (
Yeah it was to prevent flakiness in tests due to leadership change between
querying for leader master and issuing leader master RPC.
Added a functor approach instead.
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@194
PS5, Line 194: num_masters_
> nit: this number changes throughout the test, so it isn't obvious what its
Changed to orig_num_masters_ and check using orig_num_masters_ as the base.
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@198
PS5, Line 198: returning the status.
> nit: could you mention how this might fail, so it's obvious when and why we
Done
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@258
PS5, Line 258: ASSERT_EVENTUALLY
> nit: in general, it isn't obvious why we need ASSERT_EVENTUALLYs everywhere
Done
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@273
PS5, Line 273: num_masters_
> nit: I'd much rather have this be constant and explicitly use `orig_num_mas
Done
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@399
PS5, Line 399: SleepFor(MonoDelta::FromSeconds(1));
> nit: could we instead look at metrics to wait until WAL GC has happened?
I took a look around but couldn't determine the right metric that would suggest
sys catalog WAL has been GC'ed or what it's current size is to determine that
event has happened.
Added TODO and keeping the sleep here for now.
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@515
PS5, Line 515: });
> nit: could we also run VerifyNumMastersAndGetAddresses here too? Same with
Done
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@518
PS5, Line 518: a master with missing master address
> Not sure whether I missed it or not, but what about the case of trying to a
Done
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@524
PS5, Line 524: master
> nit: drop
Done
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master.h
File src/kudu/master/master.h:
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master.h@92
PS5, Line 92: MasterOptions* mutable_opts() { return &opts_; }
> Is this needed still?
Good catch.
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master.cc
File src/kudu/master/master.cc:
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master.cc@64
PS5, Line 64: namespace kudu {
: namespace rpc {
: class RpcContext;
: } // namespace rpc
: } // namespace kudu
> nit: can you move this down into the other kudu namespace?
Done
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master_service.cc
File src/kudu/master/master_service.cc:
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master_service.cc@267
PS5, Line 267: InitiateMasterChange
> nit: InitiateMasterChangeConfig
Done
--
To view, visit http://gerrit.cloudera.org:8080/16321
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c
Gerrit-Change-Number: 16321
Gerrit-PatchSet: 5
Gerrit-Owner: Bankim Bhavsar
Gerrit-Reviewer: Alexey Serbin
[kudu-CR] KUDU-3192: [client] Expose the cluster ID in the client KuduTable
Andrew Wong has removed a vote on this change. Change subject: KUDU-3192: [client] Expose the cluster ID in the client KuduTable .. Removed Code-Review+2 by Andrew Wong -- To view, visit http://gerrit.cloudera.org:8080/16493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ia1f5a451aaa44834534d2387ee1c9aa9cf95dd37 Gerrit-Change-Number: 16493 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3192: [client] Expose the cluster ID in the client KuduTable
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16493 ) Change subject: KUDU-3192: [client] Expose the cluster ID in the client KuduTable .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/16493/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16493/2//COMMIT_MSG@9 PS2, Line 9: This patch adds a cluster ID field to the KuduTable object in the I'm surprised this field has to be associated with Kudu tables instead of entire Kudu clients, which get initialized with a single cluster via the master addresses. I understand that it will be a per table field in the HMS, but I'm not convinced that means it should be a public API for tables. -- To view, visit http://gerrit.cloudera.org:8080/16493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia1f5a451aaa44834534d2387ee1c9aa9cf95dd37 Gerrit-Change-Number: 16493 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 17:44:13 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3192: [client] Expose the cluster ID in the client KuduTable
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16493 ) Change subject: KUDU-3192: [client] Expose the cluster ID in the client KuduTable .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16493/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16493/2//COMMIT_MSG@9 PS2, Line 9: This patch adds a cluster ID field to the KuduTable object in the > I'm surprised this field has to be associated with Kudu tables instead of e It is convenient to have it on the KuduTable object so that the user can check if the table is from the cluster they expect. It could also be useful if there are applications that talk to multiple clusters. I could add cluster ID to the ConnectToMasterResponsePB and store it in the client as well when it connects, but I still might consider adding a convenience method to the client that returns client.getClusterId using the client in the KuduTable object. -- To view, visit http://gerrit.cloudera.org:8080/16493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia1f5a451aaa44834534d2387ee1c9aa9cf95dd37 Gerrit-Change-Number: 16493 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 17:57:23 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3192: [client] Expose the cluster ID in the client KuduTable
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16493 ) Change subject: KUDU-3192: [client] Expose the cluster ID in the client KuduTable .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16493/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16493/2//COMMIT_MSG@9 PS2, Line 9: This patch adds a cluster ID field to the KuduTable object in the > It is convenient to have it on the KuduTable object so that the user can ch I agree it's convenient to know the cluster ID, but every table will hit the same masters and get the same cluster ID. It seems like if we have a KuduTable, we should also have a Kudu client, in which case we could get the cluster ID directly from the client for such validation. I'd be onboard with the ConnectToMaster approach. -- To view, visit http://gerrit.cloudera.org:8080/16493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia1f5a451aaa44834534d2387ee1c9aa9cf95dd37 Gerrit-Change-Number: 16493 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 18:13:57 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3192: [client] Expose the cluster ID in the client KuduTable
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16493 ) Change subject: KUDU-3192: [client] Expose the cluster ID in the client KuduTable .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia1f5a451aaa44834534d2387ee1c9aa9cf95dd37 Gerrit-Change-Number: 16493 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 18:15:36 + Gerrit-HasComments: No
[kudu-CR] KUDU-3192: [client] Expose the cluster ID in the client KuduTable
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16493 ) Change subject: KUDU-3192: [client] Expose the cluster ID in the client KuduTable .. Patch Set 2: Mobile gerrit makes it too easy to +2 things :) -- To view, visit http://gerrit.cloudera.org:8080/16493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia1f5a451aaa44834534d2387ee1c9aa9cf95dd37 Gerrit-Change-Number: 16493 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 18:16:04 + Gerrit-HasComments: No
[kudu-CR] KUDU-3192: [client] Expose the cluster ID in the client KuduTable
Andrew Wong has removed a vote on this change. Change subject: KUDU-3192: [client] Expose the cluster ID in the client KuduTable .. Removed Code-Review+2 by Andrew Wong -- To view, visit http://gerrit.cloudera.org:8080/16493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ia1f5a451aaa44834534d2387ee1c9aa9cf95dd37 Gerrit-Change-Number: 16493 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3192: [hms] Leverage the cluster ID in HMS sync
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16494 ) Change subject: KUDU-3192: [hms] Leverage the cluster ID in HMS sync .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/16494/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16494/1//COMMIT_MSG@9 PS1, Line 9: This patch propagates the cluster ID to the HMS entries for Could you also add tests where we create some real-looking metadata with no or different cluster IDs and ensure we ignore or account for it as appropriate? http://gerrit.cloudera.org:8080/#/c/16494/1/src/kudu/master/hms_notification_log_listener.cc File src/kudu/master/hms_notification_log_listener.cc: http://gerrit.cloudera.org:8080/#/c/16494/1/src/kudu/master/hms_notification_log_listener.cc@363 PS1, Line 363: before_table.tableName, cluster_id); Should dereference cluster_id http://gerrit.cloudera.org:8080/#/c/16494/1/src/kudu/master/hms_notification_log_listener.cc@437 PS1, Line 437: // If there is not a cluster ID, for maximum compatibility we should assume this is an older Uncomment this? Also maybe encapsulate in some FilterDifferentClusterId() method? -- To view, visit http://gerrit.cloudera.org:8080/16494 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I865b418a3cc4e11c889cc4757cd940831c43af17 Gerrit-Change-Number: 16494 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 23 Sep 2020 18:35:49 + Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16495 ) Change subject: WIP KUDU-2612 p2 (b): add transaction status retrieval .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG@26 PS2, Line 26: -- Is it OK to have a method to fetch transaction status I was on the fence about exposing as a CoordinatorOp because it seems like there is a difference in concerns -- all other ops may mutate the transaction state, while this new op doesn't. That said, beyond that I can't articulate a good reason why not reuse what we have. I'm fine keeping it as is. http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG@28 PS2, Line 28: -- Is it OK to use PB structures for the status of I think it's fine to expose these as pb given it is internal API. http://gerrit.cloudera.org:8080/#/c/16495/2/src/kudu/transactions/txn_status_manager-test.cc File src/kudu/transactions/txn_status_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/16495/2/src/kudu/transactions/txn_status_manager-test.cc@445 PS2, Line 445: // Supplying not-yet-used transaction idenfier. Nit: ID or identifier, same below http://gerrit.cloudera.org:8080/#/c/16495/2/src/kudu/tserver/tserver_admin.proto File src/kudu/tserver/tserver_admin.proto: http://gerrit.cloudera.org:8080/#/c/16495/2/src/kudu/tserver/tserver_admin.proto@55 PS2, Line 55: // TODO(aserbin): does it make sense to populate this with the current status Good thought. I can imagine this being useful in the future for debugging cases where we have returned an error -- To view, visit http://gerrit.cloudera.org:8080/16495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 Gerrit-Change-Number: 16495 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 19:09:24 + Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16495 ) Change subject: WIP KUDU-2612 p2 (b): add transaction status retrieval .. Patch Set 2: (1 comment) Thank you for the quick review. Just exploring an extra option: maybe, there should be a separate RPC to retrieve a status of a transaction? http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG@26 PS2, Line 26: -- Is it OK to have a method to fetch transaction status > I was on the fence about exposing as a CoordinatorOp because it seems like As a better option, do you think a separate RPC in the TabletServerAdminService interface would be more appropriate? Something like GetTransactionStatus() ? -- To view, visit http://gerrit.cloudera.org:8080/16495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 Gerrit-Change-Number: 16495 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 19:38:11 + Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/16321 )
Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master
..
Patch Set 6:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc
File src/kudu/master/dynamic_multi_master-test.cc:
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@258
PS5, Line 258: *req.mutabl
> Done
Thanks! Much clearer this time around
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc
File src/kudu/master/dynamic_multi_master-test.cc:
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@162
PS6, Line 162: }
Nit: Would it make sense to define a deadline instead and keep retrying until
then?
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@266
PS6, Line 266: return RunLeaderMasterRPC(add_master).AndThen([&] {
Nit: AndThen is useful when we need to do something extra with the result of
the first error we see (eg check for specific errors). If we're just returning
the error, it's simpler to just use RETURN_NOT_OK() and return.
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@399
PS6, Line 399: }
The log_gc_duration histogram metric has a TotalCount() method that might be
useful.
I use it here:
https://gerrit.cloudera.org/c/16492/3/src/kudu/integration-tests/txn_participant-itest.cc
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master.cc
File src/kudu/master/master.cc:
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master.cc@424
PS5, Line 424: auto
> nit: could be a const ref
Missed this?
--
To view, visit http://gerrit.cloudera.org:8080/16321
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c
Gerrit-Change-Number: 16321
Gerrit-PatchSet: 6
Gerrit-Owner: Bankim Bhavsar
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 23 Sep 2020 19:46:56 +
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16495 ) Change subject: WIP KUDU-2612 p2 (b): add transaction status retrieval .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG@26 PS2, Line 26: -- Is it OK to have a method to fetch transaction status > As a better option, do you think a separate RPC in the TabletServerAdminSer That sounds good to me too. -- To view, visit http://gerrit.cloudera.org:8080/16495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 Gerrit-Change-Number: 16495 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 20:39:37 + Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16495 ) Change subject: WIP KUDU-2612 p2 (b): add transaction status retrieval .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG@26 PS2, Line 26: -- Is it OK to have a method to fetch transaction status > That sounds good to me too. The main benefit to using CoordinatorOp is that it lets us reuse the existing CoordinatorOp code in the system client. -- To view, visit http://gerrit.cloudera.org:8080/16495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 Gerrit-Change-Number: 16495 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 20:41:40 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Greg Solovyev, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16388 to look at the new patch set (#8). Change subject: KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled .. KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled This patch enhances the HMS plugin to check if synchronization is enabled on the Kudu cluster backing the table. This can simplify enabling HMS synchronization because the plugin jar can be added to the HMS classpath unconditionally. This also helps support cases where multiple Kudu clusters use the same HMS and one is synchronized and the other isn’t. This patch introduces a new environment property, `KUDU_HMS_SYNC_ENABLED`, to the plugin which allows for faster tests that do not need to setup a Kudu cluster. It could also be useful to disable this new functionality as a workaround if issues are seen. Additonally, this patch adjusts the mini HMS to add security properties configuration. This is required now that the Kudu client runs in the plugin and communicates with the cluster. See these commits for context: - https://github.com/apache/kudu/commit/0ee93e31a1febb987c72e7392a69b2584e6f38ed - https://github.com/apache/kudu/commit/3343144fefaad5a30e95e21297c64c78e308fa1f I also manually verified the following behavior on a real cluster: - `KUDU_HMS_SYNC_ENABLED` environment variable - CREATE/ALTER/DROP on non-sync cluster works - - With plugin jar used by the HMS - CREATE/ALTER/DROP on sync cluster fails with clear message - Authn via Kerberos Change-Id: Ib3588d72af1bb499202b47fca50a08876e13ea37 --- M java/kudu-hive/build.gradle M java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java M java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java M src/kudu/hms/hms_catalog-test.cc M src/kudu/hms/hms_client-test.cc M src/kudu/hms/mini_hms.cc M src/kudu/hms/mini_hms.h 7 files changed, 239 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/16388/8 -- To view, visit http://gerrit.cloudera.org:8080/16388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib3588d72af1bb499202b47fca50a08876e13ea37 Gerrit-Change-Number: 16388 Gerrit-PatchSet: 8 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16495 ) Change subject: WIP KUDU-2612 p2 (b): add transaction status retrieval .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG@26 PS2, Line 26: -- Is it OK to have a method to fetch transaction status > The main benefit to using CoordinatorOp is that it lets us reuse the existi Right, that's what I thought to rely upon when adding the client-side bindings in TxnSystemClient. That code needs to be modified a bit, though. OK, let's then keep this new type for the CoordinatorOp. I guess we can update it further if we find this approach is not so good for some other reasons that we cannot see right away. -- To view, visit http://gerrit.cloudera.org:8080/16495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 Gerrit-Change-Number: 16495 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 20:45:58 + Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16495 ) Change subject: WIP KUDU-2612 p2 (b): add transaction status retrieval .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16495/2//COMMIT_MSG@26 PS2, Line 26: -- Is it OK to have a method to fetch transaction status > Right, that's what I thought to rely upon when adding the client-side bindi Yep, SGTM. -- To view, visit http://gerrit.cloudera.org:8080/16495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 Gerrit-Change-Number: 16495 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 20:51:38 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16388 ) Change subject: KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib3588d72af1bb499202b47fca50a08876e13ea37 Gerrit-Change-Number: 16388 Gerrit-PatchSet: 8 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Sep 2020 21:36:39 + Gerrit-HasComments: No
[kudu-CR] KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled
Grant Henke has removed a vote on this change. Change subject: KUDU-3187: Enhance the HMS plugin to check if synchronization is enabled .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ib3588d72af1bb499202b47fca50a08876e13ea37 Gerrit-Change-Number: 16388 Gerrit-PatchSet: 8 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Bankim Bhavsar has posted comments on this change. (
http://gerrit.cloudera.org:8080/16321 )
Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master
..
Patch Set 6:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/catalog_manager.cc@1615
PS6, Line 1615: optional user = rpc ?
> error: no viable conversion from 'optionalhttp://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/catalog_manager.cc@1848
PS6, Line 1848: req.has_dimension_label() ?
make_optional(req.dimension_label()) : none;
> error: no matching function for call to 'make_optional' [clang-diagnostic-e
Done
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/catalog_manager.cc@2146
PS6, Line 2146: optional user = rpc ?
> error: no viable conversion from 'optionalhttp://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/catalog_manager.cc@2506
PS6, Line 2506:
make_optional(step.add_range_partition().dimension_label()) : none;
> error: no matching function for call to 'make_optional' [clang-diagnostic-e
Done
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/catalog_manager.cc@2581
PS6, Line 2581: optional user = rpc ?
> error: no viable conversion from 'optionalhttp://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/catalog_manager.cc@4822
PS6, Line 4822: make_optional(old_info.pb.dimension_label()) :
none;
> error: no matching function for call to 'make_optional' [clang-diagnostic-e
Done
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc
File src/kudu/master/dynamic_multi_master-test.cc:
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@162
PS6, Line 162: }
> Nit: Would it make sense to define a deadline instead and keep retrying unt
Done
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@177
PS6, Line 177: return std::make_pair(std::move(s), std::move(err_code));
> warning: std::move of the variable 'err_code' of the trivially-copyable typ
Done
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@263
PS6, Line 263: return std::make_pair(std::move(s), std::move(err_code));
> warning: std::move of the variable 'err_code' of the trivially-copyable typ
Done
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@266
PS6, Line 266: return RunLeaderMasterRPC(add_master).AndThen([&] {
> Nit: AndThen is useful when we need to do something extra with the result o
Done
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@399
PS6, Line 399: }
> The log_gc_duration histogram metric has a TotalCount() method that might b
Done. Thanks!
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master.cc
File src/kudu/master/master.cc:
http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master.cc@424
PS5, Line 424: auto
> Missed this?
Indeed. Done.
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/master_service.cc
File src/kudu/master/master_service.cc:
http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/master_service.cc@435
PS6, Line 435: make_optional(rpc->remote_user().username()));
> error: no viable conversion from 'optionalhttp://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/master_service.cc@474
PS6, Line 474: req, resp, make_optional(rpc->remote_user().username()));
> error: no viable conversion from 'optionalhttp://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/master_service.cc@514
PS6, Line 514: req, resp, make_optional(rpc->remote_user().username()));
> error: no viable conversion from 'optionalhttp://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/master_service.cc@528
PS6, Line 528: req, resp, make_optional(rpc->remote_user().username()));
> error: no viable conversion from 'optionalhttp://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/master_service.cc@541
PS6, Line 541: req, resp, make_optional(rpc->remote_user().username()));
> error: no viable conversion from 'optionalhttp://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/master_service.cc@563
PS6, Line 563: req, resp, make_optional(rpc->remote_user().username()));
> error: no viable conversion from 'optionalhttp://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/master_service.cc@581
PS6, Line 581: req, resp, make_optional(rpc->remote_user().username()),
> error: no viable conversion from 'optionalhttp://gerrit.cloudera.org:8080/16321
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16321 to look at the new patch set (#7). Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master .. [master] KUDU-2181 Raft ChangeConfig request to add a master This change: - Adds hidden feature flag "--master_support_change_config" off by default. - RPC changes to add a master that initiates Raft config change and responds asynchronously. - RPC changes to report back member type(VOTER/NON_VOTER) of masters - Removes the cached master_addresses in catalog manager as it's no longer static. - Updates and adds comments in MasterOptions such that it's used to fetch master addresses only during master init time as masters can be added/removed dynamically with this change. - Updates ListMasters() to look at local Raft config instead of MasterOptions as the masters can be added/removed dynamically. If the new master can be caught up from the WAL then the master gets promoted to VOTER else it remains as NON_VOTER without master tablet copying support. Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c --- M src/kudu/common/wire_protocol.proto M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h A src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/master/master.proto M src/kudu/master/master_options.cc M src/kudu/master/master_options.h M src/kudu/master/master_path_handlers.cc M src/kudu/master/master_service.cc M src/kudu/master/master_service.h M src/kudu/master/mini_master.cc M src/kudu/master/sys_catalog.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/mini-cluster/external_mini_cluster.h M src/kudu/tools/tool_action_master.cc 20 files changed, 987 insertions(+), 56 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/16321/7 -- To view, visit http://gerrit.cloudera.org:8080/16321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c Gerrit-Change-Number: 16321 Gerrit-PatchSet: 7 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16495 to look at the new patch set (#3). Change subject: KUDU-2612 p2 (b): add transaction status retrieval .. KUDU-2612 p2 (b): add transaction status retrieval After offline discussions with Andrew, it became clear that TxnManager should provide an asynchronous interface to commit a transaction, i.e. something similar to CreateTable()/IsCreateTableDone(). To implement that, the TxnManager needs to check for the status of the transaction after initiating the commit phase by issuing corresponding call to TxnStatusManager (that's implemented as CoordinateTransaction() RPC to TabletServerAdminService with BEGIN_COMMIT_TXN operation type). This patch introduces the required server-side piece to retrieve the information on a transaction status from the TxnStatusManager. I'm planning to introduce corresponding bindings via the TxnSystemClient in a separate changelist. This is a follow-up to efd8c4f165460b7fa337b8ebd1856b10bc274311. Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 --- M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/CMakeLists.txt 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/CMakeLists.txt M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver_admin.proto 9 files changed, 165 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/16495/3 -- To view, visit http://gerrit.cloudera.org:8080/16495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 Gerrit-Change-Number: 16495 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16321 ) Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master .. Patch Set 7: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/16321/7/src/kudu/master/dynamic_multi_master-test.cc File src/kudu/master/dynamic_multi_master-test.cc: http://gerrit.cloudera.org:8080/#/c/16321/7/src/kudu/master/dynamic_multi_master-test.cc@141 PS7, Line 141: IllegalStatus nit: IllegalState -- To view, visit http://gerrit.cloudera.org:8080/16321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c Gerrit-Change-Number: 16321 Gerrit-PatchSet: 7 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 23 Sep 2020 23:09:25 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3192: [hms] Leverage the cluster ID in HMS sync
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16494 ) Change subject: KUDU-3192: [hms] Leverage the cluster ID in HMS sync .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/16494/1/src/kudu/hms/hms_client-test.cc File src/kudu/hms/hms_client-test.cc: http://gerrit.cloudera.org:8080/#/c/16494/1/src/kudu/hms/hms_client-test.cc@169 PS1, Line 169: ASSERT_TRUE(CreateTable(&client, database_name, table_name, : table_id, cluster_id).IsAlreadyPresent()); What if supplying the same table_id, but different cluster_id? What should be the behavior? I guess it should report Status::AlreadyPresent, and I think it would be great to explicitly document the expected behavior in this scenario. http://gerrit.cloudera.org:8080/#/c/16494/1/src/kudu/hms/hms_client-test.cc@179 PS1, Line 179: EXPECT_EQ(HmsClient::kManagedTable, my_table.tableType); I guess GetTable() should return HmsClient::kKuduClusterIdKey property along with others once cluster_id is introduced, no? If so, probably it make sense to make sure such property is present in the response? -- To view, visit http://gerrit.cloudera.org:8080/16494 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I865b418a3cc4e11c889cc4757cd940831c43af17 Gerrit-Change-Number: 16494 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 23 Sep 2020 23:29:54 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Alexey Serbin has removed a vote on this change. Change subject: KUDU-2612 p2 (b): add transaction status retrieval .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 Gerrit-Change-Number: 16495 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16495 ) Change subject: KUDU-2612 p2 (b): add transaction status retrieval .. Patch Set 3: Verified+1 Unrelated test failure in CatalogManagerTskITest.LeadershipChangeOnTskGeneration (ASAN) -- To view, visit http://gerrit.cloudera.org:8080/16495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4 Gerrit-Change-Number: 16495 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 24 Sep 2020 00:30:27 + Gerrit-HasComments: No
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16321 to look at the new patch set (#8). Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master .. [master] KUDU-2181 Raft ChangeConfig request to add a master This change: - Adds hidden feature flag "--master_support_change_config" off by default. - RPC changes to add a master that initiates Raft config change and responds asynchronously. - RPC changes to report back member type(VOTER/NON_VOTER) of masters - Removes the cached master_addresses in catalog manager as it's no longer static. - Updates and adds comments in MasterOptions such that it's used to fetch master addresses only during master init time as masters can be added/removed dynamically with this change. - Updates ListMasters() to look at local Raft config instead of MasterOptions as the masters can be added/removed dynamically. If the new master can be caught up from the WAL then the master gets promoted to VOTER else it remains as NON_VOTER without master tablet copying support. Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c --- M src/kudu/common/wire_protocol.proto M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h A src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/master/master.proto M src/kudu/master/master_options.cc M src/kudu/master/master_options.h M src/kudu/master/master_path_handlers.cc M src/kudu/master/master_service.cc M src/kudu/master/master_service.h M src/kudu/master/mini_master.cc M src/kudu/master/sys_catalog.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/mini-cluster/external_mini_cluster.h M src/kudu/tools/tool_action_master.cc 20 files changed, 987 insertions(+), 56 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/16321/8 -- To view, visit http://gerrit.cloudera.org:8080/16321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c Gerrit-Change-Number: 16321 Gerrit-PatchSet: 8 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16321 ) Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/16321/7/src/kudu/master/dynamic_multi_master-test.cc File src/kudu/master/dynamic_multi_master-test.cc: http://gerrit.cloudera.org:8080/#/c/16321/7/src/kudu/master/dynamic_multi_master-test.cc@141 PS7, Line 141: IllegalState > nit: IllegalState Done -- To view, visit http://gerrit.cloudera.org:8080/16321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c Gerrit-Change-Number: 16321 Gerrit-PatchSet: 8 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 24 Sep 2020 01:01:54 + Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16321 ) Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c Gerrit-Change-Number: 16321 Gerrit-PatchSet: 8 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 24 Sep 2020 01:29:46 + Gerrit-HasComments: No
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16470 ) Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps .. Patch Set 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/consensus/time_manager.h File src/kudu/consensus/time_manager.h: http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/consensus/time_manager.h@71 PS2, Line 71: queue Can you clarify which queue this is referring to? http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/consensus/time_manager.h@71 PS2, Line 71: i.e. : // AdvanceSafeTimeWithMessage() hasn't been called on the corresponding : // message) Wondering when this can happen? http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/consensus/time_manager.h@79 PS2, Line 79: leader lease Do you plan to address this in future patch? http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/consensus/time_manager.h@94 PS2, Line 94: the MVCC op registered for BEGIN_COMMIT is not : // finished when the op is applied Does this mean even when the op is applied it is not visible to the user inside the transaction? http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/consensus/time_manager.h@97 PS2, Line 97: Until the MVCC op is completed below, further snapshot scans at t where : // t > T_bc will block Will other concurrent transaction be blocked as well waiting for a timestamp be assigned? http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/tablet/ops/participant_op.h File src/kudu/tablet/ops/participant_op.h: http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/tablet/ops/participant_op.h@73 PS2, Line 73: Anchors the given 'op_id' in the WAL, ensuring that subsequent bootstraps : // of the tablet's WAL will leave the transaction in the appropriate state. The comment needs to be updated? http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/tablet/ops/participant_op.h@89 PS2, Line 89: void SetMvccOp(std::unique_ptr mvcc_op); : void ReleaseMvccOpToTxn(); Add a comment for these functions? http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/tablet/txn_participant.h File src/kudu/tablet/txn_participant.h: http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/tablet/txn_participant.h@227 PS2, Line 227: commit_op_ Add a short description for this? -- To view, visit http://gerrit.cloudera.org:8080/16470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72 Gerrit-Change-Number: 16470 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 24 Sep 2020 04:16:30 + Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16321 ) Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master .. Patch Set 8: Verified+1 Unrelated ASAN test failure in SlowTabletBootstrapTest.TestFallBehindSlowBootstrap -- To view, visit http://gerrit.cloudera.org:8080/16321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c Gerrit-Change-Number: 16321 Gerrit-PatchSet: 8 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 24 Sep 2020 05:31:54 + Gerrit-HasComments: No
[kudu-CR] [master] KUDU-2181 Raft ChangeConfig request to add a master
Bankim Bhavsar has removed a vote on this change. Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c Gerrit-Change-Number: 16321 Gerrit-PatchSet: 8 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612 p6 (c): add GetTransactionStatus() to TxnSystemClient
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16501 Change subject: KUDU-2612 p6 (c): add GetTransactionStatus() to TxnSystemClient .. KUDU-2612 p6 (c): add GetTransactionStatus() to TxnSystemClient This patch adds GetTransactionStatus() method into the interface of the TxnSystemClient class. Added corresponding test as well. This is a follow-up to cb1c2efb59373453e734074a02021f14c403257d and 0f9ff5ff043125be4a1150be0306373619b4ca89 Change-Id: I7fac7158df307d03db6a48087e7c5a16269a3bc6 --- M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/transactions/coordinator_rpc.cc M src/kudu/transactions/coordinator_rpc.h M src/kudu/transactions/txn_system_client.cc M src/kudu/transactions/txn_system_client.h M src/kudu/tserver/tablet_service.cc 6 files changed, 122 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/16501/1 -- To view, visit http://gerrit.cloudera.org:8080/16501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7fac7158df307d03db6a48087e7c5a16269a3bc6 Gerrit-Change-Number: 16501 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-2612 p6 (c): add GetTransactionStatus() to TxnSystemClient
Alexey Serbin has removed a vote on this change. Change subject: KUDU-2612 p6 (c): add GetTransactionStatus() to TxnSystemClient .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I7fac7158df307d03db6a48087e7c5a16269a3bc6 Gerrit-Change-Number: 16501 Gerrit-PatchSet: 1 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 p6 (c): add GetTransactionStatus() to TxnSystemClient
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16501 ) Change subject: KUDU-2612 p6 (c): add GetTransactionStatus() to TxnSystemClient .. Patch Set 1: Code-Review+1 unrelated test failure in HmsConfigurations/MasterFailoverTest.TestMasterPermanentFailure/1 -- To view, visit http://gerrit.cloudera.org:8080/16501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7fac7158df307d03db6a48087e7c5a16269a3bc6 Gerrit-Change-Number: 16501 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 24 Sep 2020 06:57:49 + Gerrit-HasComments: No
