Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12240 )

Change subject: KUDU-2456 Limit number of pending transactions logged
......................................................................


Patch Set 2:

(2 comments)

IWYU is a special static analysis tool that we use to manage our #include 
statements. It's very difficult to ensure that any given file has just the 
right #include statements; IWYU automates this process for us by recommending 
additions/deletions of #include statements in order to produce just the right 
#include tree.

Now, in this case it recommended adding <stddef.h> because your patch 
introduces  the size_t data type to transaction_tracker.cc for the first time. 
So it's a valid recommendation. Though I'd actually recommend adding <cstddef>; 
we generally prefer to use C++ equivalents of C headers where they exist.

http://gerrit.cloudera.org:8080/#/c/12240/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12240/2//COMMIT_MSG@14
PS2, Line 14: Tests:
            : - Pending
Since it's just a logging change, I don't think you need to explicitly unit 
test it.

If you could verify that the logging looks as you'd expect on a small cluster, 
though, that would be great.


http://gerrit.cloudera.org:8080/#/c/12240/2/src/kudu/tablet/transactions/transaction_tracker.cc
File src/kudu/tablet/transactions/transaction_tracker.cc:

http://gerrit.cloudera.org:8080/#/c/12240/2/src/kudu/tablet/transactions/transaction_tracker.cc@210
PS2, Line 210:   static const size_t kMaxTxnsToPrint = 50;
Style nit: define static constants before regular variables.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I626adfea269a15b5cadd13876c940ba5bfdcfb03
Gerrit-Change-Number: 12240
Gerrit-PatchSet: 2
Gerrit-Owner: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Greg Solovyev <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 18 Jan 2019 19:11:30 +0000
Gerrit-HasComments: Yes

Reply via email to