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/15 08:01:04 UTC

[kudu-CR] KUDU-2612: add RPC to send participant ops

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


Change subject: KUDU-2612: add RPC to send participant ops
......................................................................

KUDU-2612: add RPC to send participant ops

This adds methods to the TxnSystemClient to send participant ops to
participants by their tablet ID. This will be used in steps 13 and
18 of the transactions write path[1].

The new ParticipantRpc abstraction borrows a lot from CoordinatorRpc
with regards to lookups and error handling, with the following
differences:
- Rather than doing the lookup by table and partition key, it performs a
  lookup by tablet ID, using the functionality recently added to the
  MetaCache.
- Since TxnParticipants don't return success on repeated participant op
  requests calls, some additional handling is done for the
  TXN_OP_ALREADY_APPLIED error code.

[1] https://docs.google.com/document/d/1qv7Zejpfzg-HvF5azRL49g5lRLQ4437EmJ53GiupcWQ/edit#heading=h.4lm41o75ev1x

Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
---
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/tablet/txn_participant-test-util.h
M src/kudu/transactions/CMakeLists.txt
A src/kudu/transactions/participant_rpc.cc
A src/kudu/transactions/participant_rpc.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/transactions/txn_system_client.h
9 files changed, 750 insertions(+), 15 deletions(-)



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

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

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

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

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

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................

KUDU-2612: add RPC to send participant ops

This adds methods to the TxnSystemClient to send participant ops to
participants by their tablet ID. This will be used in steps 13 and
18 of the transactions write path[1].

The new ParticipantRpc abstraction borrows a lot from CoordinatorRpc
with regards to lookups and error handling, with the following
differences:
- Rather than doing the lookup by table and partition key, it performs a
  lookup by tablet ID, using the functionality recently added to the
  MetaCache.
- Since TxnParticipants don't return success on repeated participant op
  requests calls, some additional handling is done for the
  TXN_OP_ALREADY_APPLIED error code.

[1] https://docs.google.com/document/d/1qv7Zejpfzg-HvF5azRL49g5lRLQ4437EmJ53GiupcWQ/edit#heading=h.4lm41o75ev1x

Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
---
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/tablet/txn_participant-test-util.h
M src/kudu/transactions/CMakeLists.txt
A src/kudu/transactions/participant_rpc.cc
A src/kudu/transactions/participant_rpc.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/transactions/txn_system_client.h
9 files changed, 763 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/16879/4
-- 
To view, visit http://gerrit.cloudera.org:8080/16879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................

KUDU-2612: add RPC to send participant ops

This adds methods to the TxnSystemClient to send participant ops to
participants by their tablet ID. This will be used in steps 13 and
18 of the transactions write path[1].

The new ParticipantRpc abstraction borrows a lot from CoordinatorRpc
with regards to lookups and error handling, with the following
differences:
- Rather than doing the lookup by table and partition key, it performs a
  lookup by tablet ID, using the functionality recently added to the
  MetaCache.
- Since TxnParticipants don't return success on repeated participant op
  requests calls, some additional handling is done for the
  TXN_OP_ALREADY_APPLIED error code.

[1] https://docs.google.com/document/d/1qv7Zejpfzg-HvF5azRL49g5lRLQ4437EmJ53GiupcWQ/edit#heading=h.4lm41o75ev1x

Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Reviewed-on: http://gerrit.cloudera.org:8080/16879
Tested-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/tablet/txn_participant-test-util.h
M src/kudu/transactions/CMakeLists.txt
A src/kudu/transactions/participant_rpc.cc
A src/kudu/transactions/participant_rpc.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/transactions/txn_system_client.h
9 files changed, 806 insertions(+), 36 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................


Patch Set 4:

(8 comments)

Overall looks good to me, just few nits and a couple of question for the newly added tests scenarios.

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc
File src/kudu/integration-tests/txn_participant-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@782
PS4, Line 782: MonoDelta::FromSeconds(10)
nit for here and other test scenarios added: make this a scenario's scope-wide constant?


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@815
PS4, Line 815: BEGIN_COMMIT
Does it make sense to add a sub-scenario to show what happens when calling ABORT at this stage?  What's the expected status?


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@896
PS4, Line 896: Restart the down servers and check that we get to the right state.
What would happen if the leader replica that keeps the information about the transaction in run-time is paused, the rest two replicas are restarted and one of the restarted replicas becomes a leader, and the txn_client somehow retries and calls ParticipateInTransaction() with the same txn_id but with the new leader?  What happens what the former leader is resumed/unfrozen?  Will the state of the former leader replica will be consistent, so that on next re-election it would think of the new transaction state as of kOpen, not kInitializing?


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@1215
PS4, Line 1215: const
nit: might it be constexpr ?


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@1221
PS4, Line 1221: ParticipantOpPB::BEGIN_TXN,
              :                           ParticipantOpPB::BEGIN_COMMIT,
              :                           ParticipantOpPB::ABORT_TXN })
nit: does it make to separate this into a member field like kCommitSequence?


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/participant_rpc.h
File src/kudu/transactions/participant_rpc.h:

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/participant_rpc.h@56
PS4, Line 56:   // NOTE: if 'op_result' is non-null,
Update the comment?


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/participant_rpc.cc
File src/kudu/transactions/participant_rpc.cc:

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/participant_rpc.cc@76
PS4, Line 76:   ParticipantRpc* rpc = new ParticipantRpc(std::move(ctx),
            :                                            std::move(server_picker),
            :                                            deadline,
            :                                            std::move(user_cb),
            :                                            begin_commit_timestamp);
            :   return rpc;
nit: why not just

return new ParticipantRpc() ... ?


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/txn_system_client.cc
File src/kudu/transactions/txn_system_client.cc:

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/txn_system_client.cc@321
PS4, Line 321: ctx_raw
Since we have switched to C++17 from C++11, maybe it's time to start using move-capture here for 'ctx' as well (I guess it should have been available in C++14 already)?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
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: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 18 Dec 2020 06:44:12 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

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

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

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................

KUDU-2612: add RPC to send participant ops

This adds methods to the TxnSystemClient to send participant ops to
participants by their tablet ID. This will be used in steps 13 and
18 of the transactions write path[1].

The new ParticipantRpc abstraction borrows a lot from CoordinatorRpc
with regards to lookups and error handling, with the following
differences:
- Rather than doing the lookup by table and partition key, it performs a
  lookup by tablet ID, using the functionality recently added to the
  MetaCache.
- Since TxnParticipants don't return success on repeated participant op
  requests calls, some additional handling is done for the
  TXN_OP_ALREADY_APPLIED error code.

[1] https://docs.google.com/document/d/1qv7Zejpfzg-HvF5azRL49g5lRLQ4437EmJ53GiupcWQ/edit#heading=h.4lm41o75ev1x

Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
---
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/tablet/txn_participant-test-util.h
M src/kudu/transactions/CMakeLists.txt
A src/kudu/transactions/participant_rpc.cc
A src/kudu/transactions/participant_rpc.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/transactions/txn_system_client.h
9 files changed, 763 insertions(+), 19 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................


Patch Set 5: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc
File src/kudu/integration-tests/txn_participant-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@896
PS4, Line 896: o* leader_replica = replicas[kLeaderIdx];
> When the old leader rejoins the quorum, it will abort ops from previous ter
Thank you for the clarification.  I think this behavior sounds like a consistent one, at least at this point I don't see how it could lead to a trouble, so it looks good so far :)


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/txn_system_client.cc
File src/kudu/transactions/txn_system_client.cc:

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/txn_system_client.cc@321
PS4, Line 321: ectly.
> Alas, it seems like there still won't be a great way to pass a unique_ptr i
OK, I'll take a closer look -- thank you for the link.  It's educational.  Maybe, I'll play with it a bit, but for now this looks good enough as it is.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 22 Dec 2020 19:26:43 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................


Patch Set 5: Verified+1

(8 comments)

The test failures seem unrelated and are likely due to the recent internal failures to populate the flaky test dashboard.

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc
File src/kudu/integration-tests/txn_participant-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@782
PS4, Line 782: RT_OK(txn_client->Particip
> nit for here and other test scenarios added: make this a scenario's scope-w
Done


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@815
PS4, Line 815: 
> Does it make sense to add a sub-scenario to show what happens when calling 
Done


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@896
PS4, Line 896: o* leader_replica = replicas[kLeaderIdx];
> What would happen if the leader replica that keeps the information about th
When the old leader rejoins the quorum, it will abort ops from previous terms, and the abort path for participant ops includes a call to ClearIfInitFailed, which would remove the Txn rather than leaving it kInitializing.


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@1215
PS4, Line 1215: 
> nit: might it be constexpr ?
Done


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/integration-tests/txn_participant-itest.cc@1221
PS4, Line 1221: plica> r;
              :     ASSERT_TRUE(cluster_->mini_tablet_server(i)->server()->tablet_manager()->LookupTablet(
              :         tablet_id, &r));
> nit: does it make to separate this into a member field like kCommitSequence
Done


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/participant_rpc.h
File src/kudu/transactions/participant_rpc.h:

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/participant_rpc.h@56
PS4, Line 56:   // NOTE: if 'begin_commit_timestamp'
> Update the comment?
Done


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/participant_rpc.cc
File src/kudu/transactions/participant_rpc.cc:

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/participant_rpc.cc@76
PS4, Line 76:   return new ParticipantRpc(std::move(ctx),
            :                             std::move(server_picker),
            :                             deadline,
            :                             std::move(user_cb),
            :                             begin_commit_timestamp);
            : }
> nit: why not just
Done


http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/txn_system_client.cc
File src/kudu/transactions/txn_system_client.cc:

http://gerrit.cloudera.org:8080/#/c/16879/4/src/kudu/transactions/txn_system_client.cc@321
PS4, Line 321: ectly.
> Since we have switched to C++17 from C++11, maybe it's time to start using 
Alas, it seems like there still won't be a great way to pass a unique_ptr into a lambda:
https://taylorconor.com/blog/noncopyable-lambdas/

I tried some stuff with lambdas, but haven't hammered on it enough to get to the bottom of how to do this. I'll leave a TODO and this link as a comment.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 21 Dec 2020 01:17:05 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16879/3/src/kudu/transactions/txn_system_client.cc
File src/kudu/transactions/txn_system_client.cc:

http://gerrit.cloudera.org:8080/#/c/16879/3/src/kudu/transactions/txn_system_client.cc@330
PS3, Line 330:             std::move(cb),
> warning: std::move of the const variable 'cb' has no effect; remove std::mo
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 15 Dec 2020 19:47:29 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

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

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

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................

KUDU-2612: add RPC to send participant ops

This adds methods to the TxnSystemClient to send participant ops to
participants by their tablet ID. This will be used in steps 13 and
18 of the transactions write path[1].

The new ParticipantRpc abstraction borrows a lot from CoordinatorRpc
with regards to lookups and error handling, with the following
differences:
- Rather than doing the lookup by table and partition key, it performs a
  lookup by tablet ID, using the functionality recently added to the
  MetaCache.
- Since TxnParticipants don't return success on repeated participant op
  requests calls, some additional handling is done for the
  TXN_OP_ALREADY_APPLIED error code.

[1] https://docs.google.com/document/d/1qv7Zejpfzg-HvF5azRL49g5lRLQ4437EmJ53GiupcWQ/edit#heading=h.4lm41o75ev1x

Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
---
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/tablet/txn_participant-test-util.h
M src/kudu/transactions/CMakeLists.txt
A src/kudu/transactions/participant_rpc.cc
A src/kudu/transactions/participant_rpc.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/transactions/txn_system_client.h
9 files changed, 763 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/16879/3
-- 
To view, visit http://gerrit.cloudera.org:8080/16879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................


Patch Set 1:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:

http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/client/meta_cache.cc@505
PS1, Line 505:   // TODO: When we support tablet splits, we should let the lookup shift
> warning: missing username/bug in TODO [google-readability-todo]
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/integration-tests/txn_participant-itest.cc
File src/kudu/integration-tests/txn_participant-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/integration-tests/txn_participant-itest.cc@99
PS1, Line 99: using kudu::tablet::TabletStatePB;
> warning: using decl 'TabletStatePB' is unused [misc-unused-using-decls]
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/integration-tests/txn_participant-itest.cc@100
PS1, Line 100: using kudu::tablet::Txn;
> warning: using decl 'Txn' is unused [misc-unused-using-decls]
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/tablet/txn_participant-test-util.h
File src/kudu/tablet/txn_participant-test-util.h:

http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/tablet/txn_participant-test-util.h@38
PS1, Line 38: tserver::ParticipantOpPB MakeParticipantOp(
> warning: function 'MakeParticipantOp' defined in a header file; function de
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/tablet/txn_participant-test-util.h@51
PS1, Line 51: std::unique_ptr<ParticipantOpState> NewParticipantOp(
> warning: function 'NewParticipantOp' defined in a header file; function def
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/transactions/participant_rpc.cc
File src/kudu/transactions/participant_rpc.cc:

http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/transactions/participant_rpc.cc@39
PS1, Line 39: using kudu::client::internal::RemoteTablet;
> warning: using decl 'RemoteTablet' is unused [misc-unused-using-decls]
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/transactions/participant_rpc.cc@41
PS1, Line 41: using kudu::client::KuduClient;
> warning: using decl 'KuduClient' is unused [misc-unused-using-decls]
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/transactions/participant_rpc.cc@48
PS1, Line 48: using kudu::tserver::CoordinatorOpResultPB;
> warning: using decl 'CoordinatorOpResultPB' is unused [misc-unused-using-de
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/transactions/txn_system_client.h
File src/kudu/transactions/txn_system_client.h:

http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/transactions/txn_system_client.h@139
PS1, Line 139:   Status ParticipateInTransaction(const std::string& tablet_id,
> warning: function 'kudu::transactions::TxnSystemClient::ParticipateInTransa
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/transactions/txn_system_client.h@161
PS1, Line 161:   void ParticipateInTransactionAsync(const std::string& tablet_id,
> warning: function 'kudu::transactions::TxnSystemClient::ParticipateInTransa
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/transactions/txn_system_client.cc
File src/kudu/transactions/txn_system_client.cc:

http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/transactions/txn_system_client.cc@315
PS1, Line 315:           std::move(participant_op),
> warning: std::move of the const variable 'participant_op' has no effect; re
Done


http://gerrit.cloudera.org:8080/#/c/16879/1/src/kudu/transactions/txn_system_client.cc@330
PS1, Line 330:             std::move(cb),
> warning: std::move of the const variable 'cb' has no effect; remove std::mo
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 15 Dec 2020 09:31:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................


Patch Set 4:

Failure was a LINT error, but this is otherwise ready for review.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
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: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 15 Dec 2020 20:31:30 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2612: add RPC to send participant ops

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

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

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

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

Change subject: KUDU-2612: add RPC to send participant ops
......................................................................

KUDU-2612: add RPC to send participant ops

This adds methods to the TxnSystemClient to send participant ops to
participants by their tablet ID. This will be used in steps 13 and
18 of the transactions write path[1].

The new ParticipantRpc abstraction borrows a lot from CoordinatorRpc
with regards to lookups and error handling, with the following
differences:
- Rather than doing the lookup by table and partition key, it performs a
  lookup by tablet ID, using the functionality recently added to the
  MetaCache.
- Since TxnParticipants don't return success on repeated participant op
  requests calls, some additional handling is done for the
  TXN_OP_ALREADY_APPLIED error code.

[1] https://docs.google.com/document/d/1qv7Zejpfzg-HvF5azRL49g5lRLQ4437EmJ53GiupcWQ/edit#heading=h.4lm41o75ev1x

Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
---
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/tablet/txn_participant-test-util.h
M src/kudu/transactions/CMakeLists.txt
A src/kudu/transactions/participant_rpc.cc
A src/kudu/transactions/participant_rpc.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/transactions/txn_system_client.h
9 files changed, 806 insertions(+), 36 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/16879/5
-- 
To view, visit http://gerrit.cloudera.org:8080/16879
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb9ba09104761772f9aaffe582776ad34d8dbf57
Gerrit-Change-Number: 16879
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)