You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2020/12/08 04:45:47 UTC

[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors 

Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16831


Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors
......................................................................

txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors

I have a patch coming up that sends RPCs that submit participant ops to
the TxnParticipant and retries if necessary. One of the criteria to
retry an RPC is checking if the Raft application returned an
IllegalState error, e.g. in RaftConsensus::CheckLeadershipAndBindTerm.

This is problematic, as TxnParticipants may also return IllegalState if
the transaction is not in an appropriate state (e.g. an RPC tries to
abort an already-committed transaction). To disambiguate such cases,
this patch updates the TxnParticipants to also set a TabletServerErrorPB
code when such an error occurs, allowing for more specific error
handling to be determined.

Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
---
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/tablet/ops/participant_op.cc
M src/kudu/tablet/ops/participant_op.h
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tablet/txn_participant.h
5 files changed, 157 insertions(+), 88 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/16831/1
-- 
To view, visit http://gerrit.cloudera.org:8080/16831
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
Gerrit-Change-Number: 16831
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>

[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.

Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/16831
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
Gerrit-Change-Number: 16831
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16831 )

Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16831
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
Gerrit-Change-Number: 16831
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Dec 2020 06:34:44 +0000
Gerrit-HasComments: No

[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16831 )

Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors
......................................................................


Patch Set 1:

It seems IWYU isn't yet happy, but otherwise LGTM.


-- 
To view, visit http://gerrit.cloudera.org:8080/16831
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
Gerrit-Change-Number: 16831
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Dec 2020 06:35:25 +0000
Gerrit-HasComments: No

[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16831 )

Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16831
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
Gerrit-Change-Number: 16831
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Dec 2020 18:09:36 +0000
Gerrit-HasComments: No

[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16831 )

Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16831
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
Gerrit-Change-Number: 16831
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Dec 2020 06:51:25 +0000
Gerrit-HasComments: No

[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16831

to look at the new patch set (#2).

Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors
......................................................................

txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors

I have a patch coming up that sends RPCs that submit participant ops to
the TxnParticipant and retries if necessary. One of the criteria to
retry an RPC is checking if the Raft application returned an
IllegalState error, e.g. in RaftConsensus::CheckLeadershipAndBindTerm.

This is problematic, as TxnParticipants may also return IllegalState if
the transaction is not in an appropriate state (e.g. an RPC tries to
abort an already-committed transaction). To disambiguate such cases,
this patch updates the TxnParticipants to also set a TabletServerErrorPB
code when such an error occurs, allowing for more specific error
handling to be determined.

Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
---
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/tablet/ops/participant_op.cc
M src/kudu/tablet/ops/participant_op.h
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tablet/txn_participant.h
5 files changed, 159 insertions(+), 88 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/16831/2
-- 
To view, visit http://gerrit.cloudera.org:8080/16831
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
Gerrit-Change-Number: 16831
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16831 )

Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors
......................................................................


Patch Set 3: Verified+1

Test failure was unrelated:
	dynamic_multi_master-test


-- 
To view, visit http://gerrit.cloudera.org:8080/16831
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
Gerrit-Change-Number: 16831
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Dec 2020 21:14:48 +0000
Gerrit-HasComments: No

[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16831 )

Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors
......................................................................

txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors

I have a patch coming up that sends RPCs that submit participant ops to
the TxnParticipant and retries if necessary. One of the criteria to
retry an RPC is checking if the Raft application returned an
IllegalState error, e.g. in RaftConsensus::CheckLeadershipAndBindTerm.

This is problematic, as TxnParticipants may also return IllegalState if
the transaction is not in an appropriate state (e.g. an RPC tries to
abort an already-committed transaction). To disambiguate such cases,
this patch updates the TxnParticipants to also set a TabletServerErrorPB
code when such an error occurs, allowing for more specific error
handling to be determined.

Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
Reviewed-on: http://gerrit.cloudera.org:8080/16831
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/tablet/ops/participant_op.cc
M src/kudu/tablet/ops/participant_op.h
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tablet/txn_participant.h
5 files changed, 159 insertions(+), 88 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Andrew Wong: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/16831
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf
Gerrit-Change-Number: 16831
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)