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/09/17 21:12:51 UTC
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16470
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.h
13 files changed, 420 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/16470/1
--
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: newchange
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
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/16470
to look at the new patch set (#5).
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.h
13 files changed, 436 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/16470/5
--
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: newpatchset
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
Gerrit-PatchSet: 5
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 p10: have timestamp assignment account for commit timestamps
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/16470
to look at the new patch set (#8).
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.h
13 files changed, 476 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/16470/8
--
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: newpatchset
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
Gerrit-PatchSet: 8
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 p10: have timestamp assignment account for commit timestamps
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16470
to look at the new patch set (#11).
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.cc
M src/kudu/tablet/txn_participant.h
14 files changed, 487 insertions(+), 83 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/16470/11
--
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: newpatchset
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
Gerrit-PatchSet: 11
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)
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong 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 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16470/1/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:
http://gerrit.cloudera.org:8080/#/c/16470/1/src/kudu/tablet/tablet_bootstrap.cc@1564
PS1, Line 1564: return AppendCommitMsg(commit_msg);
use Log::Append() directly with commit_entry.
--
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: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 17 Sep 2020 21:17:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16470 )
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Reviewed-on: http://gerrit.cloudera.org:8080/16470
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.cc
M src/kudu/tablet/txn_participant.h
14 files changed, 487 insertions(+), 83 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
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: merged
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
Gerrit-PatchSet: 12
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)
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong 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 10:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/consensus/time_manager-test.cc
File src/kudu/consensus/time_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/consensus/time_manager-test.cc@226
PS9, Line 226: {
: // Oper
> nit: I'm not sure this adds any clarity on the expected behavior. Could yo
Done
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/consensus/time_manager-test.cc@245
PS9, Line 245: // the safe time to pass.
> nit: does it make sense to check for the result of TimeManager::AssignTimes
Done
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/consensus/time_manager.h
File src/kudu/consensus/time_manager.h:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/consensus/time_manager.h@159
PS9, Line 159: Returns a not-OK status if called while not the leader
> nit: any reason not to mention the case when updating clock fails or that's
Done
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/ops/participant_op.h
File src/kudu/tablet/ops/participant_op.h:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/ops/participant_op.h@75
PS9, Line 75: messag
> message
Done
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/ops/participant_op.cc
File src/kudu/tablet/ops/participant_op.cc:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/ops/participant_op.cc@114
PS9, Line 114: DCHECK(nullptr == begin_commit_mvcc_op_);
> Is this assumed to happen only once during ParticipantOpState's lifecycle?
Done
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/tablet.cc@857
PS9, Line 857: if (op_type == tserver::ParticipantOpPB::FINALIZE_COMMIT) {
: // NOTE: we may not have an MVCC op if we are bootstrapping and did not
: // replay a BEGIN_COMMIT op.
: i
> Is it even possible to not have commit op for FINALIZE_COMMIT operation at
Yeah, when bootstrapping. I'll leave a comment.
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/txn_participant.h
File src/kudu/tablet/txn_participant.h:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/txn_participant.h@183
PS9, Line 183:
> Why not just SetCommitOp() as for ParticipantOpState::SetMvccOp()? Why doe
Done
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/txn_participant.h@228
PS9, Line 228: his ensures scans on this participant are
: // repeatable.
> nit: maybe, add some DCHECK-ing into the destructor of this Txn class to en
Done
--
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: 10
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-Comment-Date: Tue, 06 Oct 2020 07:00:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong 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 3:
(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: re ha
> Can you clarify which queue this is referring to?
Done
http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/consensus/time_manager.h@71
PS2, Line 71: been a op that
: // was assigned a timestamp that is not yet known by the queue (i.e.
: // AdvanceS
> Wondering when this can happen?
I mentioned it in the above blurb:
// - TimeManger::AdvanceSafeTimeWithMessage() is called with the replicate
// message of the op with timestamp T, signifying the new safe time.
I tried to clarify a bit. Let me know if it's still unclear.
http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/consensus/time_manager.h@79
PS2, Line 79:
> Do you plan to address this in future patch?
If you are asking if this is relevant to transactions, no; this is a separate feature from transactions.
http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/consensus/time_manager.h@94
PS2, Line 94: steps, denoting T_bc as safe.
: // - Unlike a regular (e.g. write) o
> Does this mean even when the op is applied it is not visible to the user in
This "op" doesn't correspond to a mutation -- it corresponds to a ScopedOp object, used solely to ensure we wait before we scan, per the below blurb.
// - Until the MVCC op is completed below, further snapshot scans at t where
// t > T_bc will block.
http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/consensus/time_manager.h@97
PS2, Line 97: intained in memory for the time being.
: // - Until the MVCC op i
> Will other concurrent transaction be blocked as well waiting for a timestam
Kind of, but we don't block writers, we block readers. I.e. once we begin committing, replicating the BEGIN_COMMIT op at T_bc, if we can write a row in a separate op, assigning it a timestamp T_w > T_bc. However, if we wanted to read that row, we would have to wait for all ops before T_w to finish applying, implying that we would have to wait for the FINALIZE_COMMIT op to complete.
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?
Not sure if you're referring to something specific to 'op_id'; this is all true.
Added a blurb about 'commit_msg'.
http://gerrit.cloudera.org:8080/#/c/16470/2/src/kudu/tablet/ops/participant_op.h@89
PS2, Line 89: }
:
> Add a comment for these functions?
Done
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:
> Add a short description for this?
Done
--
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: 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: Mon, 28 Sep 2020 23:15:58 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
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/16470
to look at the new patch set (#7).
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.h
13 files changed, 467 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/16470/7
--
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: newpatchset
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
Gerrit-PatchSet: 7
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 p10: have timestamp assignment account for commit timestamps
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/16470
to look at the new patch set (#3).
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.h
13 files changed, 433 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/16470/3
--
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: newpatchset
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
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 p10: have timestamp assignment account for commit timestamps
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/16470
to look at the new patch set (#9).
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.h
13 files changed, 478 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/16470/9
--
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: newpatchset
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
Gerrit-PatchSet: 9
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 p10: have timestamp assignment account for commit timestamps
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
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: deleteVote
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
Gerrit-PatchSet: 9
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 p10: have timestamp assignment account for commit timestamps
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/16470
to look at the new patch set (#6).
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.h
13 files changed, 454 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/16470/6
--
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: newpatchset
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
Gerrit-PatchSet: 6
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 p10: have timestamp assignment account for commit timestamps
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin 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 9:
(8 comments)
Overall looks good, just a few nits and questions.
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/consensus/time_manager-test.cc
File src/kudu/consensus/time_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/consensus/time_manager-test.cc@226
PS9, Line 226: // If we update the clock with a timestamp in the future, we should jump
: // ahead.
nit: I'm not sure this adds any clarity on the expected behavior. Could you add more colors on this or just drop this piece?
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/consensus/time_manager-test.cc@245
PS9, Line 245: }
nit: does it make sense to check for the result of TimeManager::AssignTimestamp() right after the last call to TimeManager::AdvanceSafeTimeWithMessage() or it doesn't constitute any invariant to be aware of?
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/consensus/time_manager.h
File src/kudu/consensus/time_manager.h:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/consensus/time_manager.h@159
PS9, Line 159: Returns a not-OK status if called while not the leader
nit: any reason not to mention the case when updating clock fails or that's just considered very unlikely scenario to mention?
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/ops/participant_op.h
File src/kudu/tablet/ops/participant_op.h:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/ops/participant_op.h@75
PS9, Line 75: messag
message
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/ops/participant_op.cc
File src/kudu/tablet/ops/participant_op.cc:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/ops/participant_op.cc@114
PS9, Line 114: begin_commit_mvcc_op_ = std::move(mvcc_op);
Is this assumed to happen only once during ParticipantOpState's lifecycle? If so, maybe add DCHECK(begin_commit_mvcc_op_.get() == nullptr) or alike?
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/tablet.cc@857
PS9, Line 857: if (op_type == tserver::ParticipantOpPB::FINALIZE_COMMIT) {
: if (op_state->txn()->commit_op()) {
: op_state->txn()->commit_op()->StartApplying();
: }
Is it even possible to not have commit op for FINALIZE_COMMIT operation at this stage?
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/txn_participant.h
File src/kudu/tablet/txn_participant.h:
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/txn_participant.h@183
PS9, Line 183: AcquireCommitOp
Why not just SetCommitOp() as for ParticipantOpState::SetMvccOp()? Why does 'acquire' is so important here to signify (add a comment about this)?
http://gerrit.cloudera.org:8080/#/c/16470/9/src/kudu/tablet/txn_participant.h@228
PS9, Line 228: spans between the BEGIN_COMMIT op and
: // corresponding FINALIZE_COMMIT or ABORT_TXN op
nit: maybe, add some DCHECK-ing into the destructor of this Txn class to enforce the described invariants?
--
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: 9
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-Comment-Date: Mon, 05 Oct 2020 23:23:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
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/16470
to look at the new patch set (#4).
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.h
13 files changed, 439 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/16470/4
--
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: newpatchset
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
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 p10: have timestamp assignment account for commit timestamps
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin 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 11: Code-Review+2
--
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: 11
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-Comment-Date: Wed, 07 Oct 2020 04:54:00 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong 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 9: Verified+1
Failures appear to be due to the cluster ID raciness that Grant is fixing.
--
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: 9
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, 01 Oct 2020 04:54:53 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16470
to look at the new patch set (#10).
Change subject: KUDU-2612 p10: have timestamp assignment account for commit timestamps
......................................................................
KUDU-2612 p10: have timestamp assignment account for commit timestamps
One of the hurdles to performing a transaction's commit on a participant
is that the commit process must ensure repeatable reads. Without
multi-op transactions, this is done via a dance between the TimeManager
(the entity that tracks and assigns timestamps) and the MvccManager (the
entity that tracks the lifecycle of ops).
This patch extends the dance between the TimeManager and MvccManager to
ensure that when a participant commits a transaction, all future ops
will be assigned a higher timestamp.
I found it time-consuming to peruse the existing codebase for how
timestamp assignment ensured repeatable reads, so I added a block
comment to time_manager.h describing how it does so. I also added a
section about how this patch extends it to ensure repeatable reads in
the context of transactions.
Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
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/ops/write_op.cc
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/txn_participant.cc
M src/kudu/tablet/txn_participant.h
14 files changed, 483 insertions(+), 83 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/16470/10
--
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: newpatchset
Gerrit-Change-Id: I0412fd0cf778d96f3fe6b14624d8d06942f40e72
Gerrit-Change-Number: 16470
Gerrit-PatchSet: 10
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)
[kudu-CR] KUDU-2612 p10: have timestamp assignment account for commit timestamps
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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<ScopedOp> 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 <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, 24 Sep 2020 04:16:30 +0000
Gerrit-HasComments: Yes