Bankim Bhavsar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16043 )

Change subject: KUDU-2612 p1: add initial transaction status storage
......................................................................


Patch Set 2:

(5 comments)

Just took a high-level look. Some comments below.

http://gerrit.cloudera.org:8080/#/c/16043/2/src/kudu/transactions/status_tablet.h
File src/kudu/transactions/status_tablet.h:

http://gerrit.cloudera.org:8080/#/c/16043/2/src/kudu/transactions/status_tablet.h@45
PS2, Line 45:   virtual Status VisitTransactionEntries(int64_t txn_id, 
std::string user,
Could you add a comment about this function?


http://gerrit.cloudera.org:8080/#/c/16043/2/src/kudu/transactions/status_tablet.h@63
PS2, Line 63: it's
            : //   easier to get the lowest value than it is to get the highest 
value
I don't understand why getting lowest value is easier than getting highest 
value.


http://gerrit.cloudera.org:8080/#/c/16043/2/src/kudu/transactions/status_tablet.h@96
PS2, Line 96:   // Uses the given visitor to iterate over the entries in the 
rows of the
            :   // underlying tablet replica.
I'll take a look below but I don't understand the purpose of this function.


http://gerrit.cloudera.org:8080/#/c/16043/2/src/kudu/transactions/status_tablet.cc
File src/kudu/transactions/status_tablet.cc:

http://gerrit.cloudera.org:8080/#/c/16043/2/src/kudu/transactions/status_tablet.cc@78
PS2, Line 78: oid ExtractKeys
Any reason to keep these as standalone functions v/s making them private static 
functions of the TxnStatusTablet class?


http://gerrit.cloudera.org:8080/#/c/16043/2/src/kudu/transactions/status_tablet.cc@193
PS2, Line 193: std::make_pair(
std::make_pair shouldn't be needed with emplace_back



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94ddbd37c65932120835d6e138307f819935173c
Gerrit-Change-Number: 16043
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 09 Jun 2020 22:12:39 +0000
Gerrit-HasComments: Yes

Reply via email to