You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2020/09/23 06:29:00 UTC
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16495
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.
This is a follow-up to efd8c4f165460b7fa337b8ebd1856b10bc274311.
WIP:
* I'd like to get some feedback on this approach
* add tests into the TxnStatusManagerTest suite
Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4
---
M src/kudu/tablet/txn_coordinator.h
M src/kudu/transactions/CMakeLists.txt
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
8 files changed, 56 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/16495/1
--
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: newchange
Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4
Gerrit-Change-Number: 16495
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Sep 2020 19:09:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Sep 2020 20:51:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 24 Sep 2020 00:30:27 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16495 )
Change subject: KUDU-2612 p2 (b): add transaction status retrieval
......................................................................
Patch Set 4: Code-Review+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: comment
Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4
Gerrit-Change-Number: 16495
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 28 Sep 2020 19:08:30 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16495/3/src/kudu/transactions/txn_status_manager-test.cc
File src/kudu/transactions/txn_status_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/16495/3/src/kudu/transactions/txn_status_manager-test.cc@405
PS3, Line 405: // Make the TxnManager start from scratch.
> nit: TxnStatusManager
Done
http://gerrit.cloudera.org:8080/#/c/16495/3/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/16495/3/src/kudu/tserver/tablet_service.cc@1278
PS3, Line 1278: } else if (op.type() == CoordinatorOpPB::GET_TXN_STATUS) {
> Could you also add some test coverage for this, maybe in TxnStatusTabletMan
Done
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 28 Sep 2020 19:03:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Sep 2020 20:39:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Sep 2020 20:41:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16495 )
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
Reviewed-on: http://gerrit.cloudera.org:8080/16495
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
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
10 files changed, 169 insertions(+), 9 deletions(-)
Approvals:
Andrew Wong: Looks good to me, approved
Kudu Jenkins: Verified
--
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: merged
Gerrit-Change-Id: I45f099d943f2b7955d6d561a1cb883343c7b79a4
Gerrit-Change-Number: 16495
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Sep 2020 19:38:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong 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:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16495/3/src/kudu/transactions/txn_status_manager-test.cc
File src/kudu/transactions/txn_status_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/16495/3/src/kudu/transactions/txn_status_manager-test.cc@405
PS3, Line 405: // Make the TxnManager start from scratch.
nit: TxnStatusManager
http://gerrit.cloudera.org:8080/#/c/16495/3/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/16495/3/src/kudu/tserver/tablet_service.cc@1278
PS3, Line 1278: } else if (op.type() == CoordinatorOpPB::GET_TXN_STATUS) {
Could you also add some test coverage for this, maybe in TxnStatusTabletManagementTest::TestTabletServerProxyCalls?
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 27 Sep 2020 19:49:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Sep 2020 20:45:58 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612 p2 (b): add transaction status retrieval
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#4).
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/integration-tests/ts_tablet_manager-itest.cc
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
10 files changed, 169 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/16495/4
--
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: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)