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 <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 24 Sep 2020 04:16:30 +0000
Gerrit-HasComments: Yes

Reply via email to