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/10/09 07:05:28 UTC
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16569
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
KUDU-2612: persist commit MVCC op timestamp in txn metadata
We previously didn't persist the MVCC op timestamp used for the
BEGIN_COMMIT op. This was problematic, as this timestamp is necessary to
determine relevancy upon iteration.
This adds the timestamp to the TxnMetadata, and ensures that we anchor
the BEGIN_COMMIT op until we flush metadata, as we do with other
persistent participant op state.
Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
---
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/ops/participant_op.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/txn_metadata.h
M src/kudu/tablet/txn_participant-test.cc
9 files changed, 180 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/16569/1
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16569 )
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
KUDU-2612: persist commit MVCC op timestamp in txn metadata
We previously didn't persist the MVCC op timestamp used for the
BEGIN_COMMIT op. This was problematic, as this timestamp is necessary to
determine relevancy upon iteration.
This adds the timestamp to the TxnMetadata, and ensures that we anchor
the BEGIN_COMMIT op until we flush metadata, as we do with other
persistent participant op state.
Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Reviewed-on: http://gerrit.cloudera.org:8080/16569
Tested-by: Kudu Jenkins
Reviewed-by: Hao Hao <ha...@cloudera.com>
---
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/ops/participant_op.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/txn_metadata.h
M src/kudu/tablet/txn_participant-test.cc
9 files changed, 180 insertions(+), 9 deletions(-)
Approvals:
Kudu Jenkins: Verified
Hao Hao: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16569 )
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16569/1/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/16569/1/src/kudu/tablet/tablet_metadata.cc@817
PS1, Line 817: if we are bootstrapping
> Wondering In which cases the mvcc op for a txn can be already persisted but
This happens when we are bootstrapping and replay a BEGIN_COMMIT op, since we have to replay WAL ops to rebuild in-memory state. Take this example:
BEGIN_TXN 1 (txn metadata is added in tablet metadata)
BEGIN_COMMIT 1 (op timestamp is updated in txn metadata)
FLUSH (txn metadata is persisted)
RESTART
In this scenario, after restarting, we see that we have an in-flight (not finalized or aborted) txn 1 persisted in metadata, so we create a new Txn object to indicate that there is an on-going Txn. Then we replay all ops for it, including the BEGIN_COMMIT op to get the participant's state to kCommitInProgress. But we don't need to anchor the WAL for this metadata flush because the metadata has already been persisted.
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 19 Oct 2020 20:41:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16569 )
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16569/2/src/kudu/tablet/metadata.proto
File src/kudu/tablet/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/16569/2/src/kudu/tablet/metadata.proto@100
PS2, Line 100: When iterating through mutations at the latest snapshot (as in READ_LATEST
: // or during compactions), this MVCC op timestamp must be applied and there
: // must be a commit timestamp for the mutation to be considered committed --
: // this avoids reading d
> Maybe nice to point out this is to avoid dirty reads?
Done
http://gerrit.cloudera.org:8080/#/c/16569/2/src/kudu/tablet/txn_participant-test.cc
File src/kudu/tablet/txn_participant-test.cc:
http://gerrit.cloudera.org:8080/#/c/16569/2/src/kudu/tablet/txn_participant-test.cc@614
PS2, Line 614: // One anchor should be gone and another should be registered in its place
: // that lasts until we flush the finalized metadata.
: ASSERT_EQ(2, tablet_replica_->log_anchor_registry()->GetAnchorCountForTests());
> The comment doesn't seem to match the checking? Why not 'ASSERT_EQ(1, ...)'
There were previously two anchors. We removed one, and added one, so we're left with 2, from L602.
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 27 Oct 2020 20:27:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16569
to look at the new patch set (#3).
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
KUDU-2612: persist commit MVCC op timestamp in txn metadata
We previously didn't persist the MVCC op timestamp used for the
BEGIN_COMMIT op. This was problematic, as this timestamp is necessary to
determine relevancy upon iteration.
This adds the timestamp to the TxnMetadata, and ensures that we anchor
the BEGIN_COMMIT op until we flush metadata, as we do with other
persistent participant op state.
Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
---
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/ops/participant_op.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/txn_metadata.h
M src/kudu/tablet/txn_participant-test.cc
9 files changed, 180 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/16569/3
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
Removed -Verified by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16569 )
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
Patch Set 2: Code-Review+1
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16569/2/src/kudu/tablet/metadata.proto
File src/kudu/tablet/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/16569/2/src/kudu/tablet/metadata.proto@100
PS2, Line 100: When iterating through mutations at the latest snapshot (as in
: // READ_LATEST or during compactions), this MVCC op timestamp must be
: // applied and there must be a commit timestamp for the mutation to be
: // considered committed.
Maybe nice to point out this is to avoid dirty reads?
http://gerrit.cloudera.org:8080/#/c/16569/1/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/16569/1/src/kudu/tablet/tablet_metadata.cc@817
PS1, Line 817: if we are bootstrapping
> This happens when we are bootstrapping and replay a BEGIN_COMMIT op, since
Ack
http://gerrit.cloudera.org:8080/#/c/16569/2/src/kudu/tablet/txn_participant-test.cc
File src/kudu/tablet/txn_participant-test.cc:
http://gerrit.cloudera.org:8080/#/c/16569/2/src/kudu/tablet/txn_participant-test.cc@614
PS2, Line 614: // One anchor should be gone and another should be registered in its place
: // that lasts until we flush the finalized metadata.
: ASSERT_EQ(2, tablet_replica_->log_anchor_registry()->GetAnchorCountForTests());
The comment doesn't seem to match the checking? Why not 'ASSERT_EQ(1, ...)'
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 26 Oct 2020 23:07:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16569 )
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
Patch Set 2: Verified+1
Unrelated flaky failure.
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 24 Oct 2020 00:23:39 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16569 )
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16569/1/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/16569/1/src/kudu/tablet/tablet_metadata.cc@817
PS1, Line 817: if we are bootstrapping
Wondering In which cases the mvcc op for a txn can be already persisted but the txn hasn't begin commit?
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 19 Oct 2020 20:19:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: persist commit MVCC op timestamp in txn metadata
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16569 )
Change subject: KUDU-2612: persist commit MVCC op timestamp in txn metadata
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16569/2/src/kudu/tablet/txn_participant-test.cc
File src/kudu/tablet/txn_participant-test.cc:
http://gerrit.cloudera.org:8080/#/c/16569/2/src/kudu/tablet/txn_participant-test.cc@614
PS2, Line 614: // One anchor should be gone and another should be registered in its place
: // that lasts until we flush the finalized metadata.
: ASSERT_EQ(2, tablet_replica_->log_anchor_registry()->GetAnchorCountForTests());
> There were previously two anchors. We removed one, and added one, so we're
Ack
--
To view, visit http://gerrit.cloudera.org:8080/16569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2400fbb7e96ddba78544a9549965fc095e32ca3
Gerrit-Change-Number: 16569
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 29 Oct 2020 01:18:25 +0000
Gerrit-HasComments: Yes