Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16400 )
Change subject: KUDU-3012: Add a Log Throttler ...................................................................... Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/16400/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16400/4//COMMIT_MSG@9 PS4, Line 9: LogThrottler uses an instance of Logger > This is part of implementation detail and unnecessary. Done http://gerrit.cloudera.org:8080/#/c/16400/4//COMMIT_MSG@11 PS4, Line 11: message. > message parameters, right? not sure I understand http://gerrit.cloudera.org:8080/#/c/16400/4/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java: http://gerrit.cloudera.org:8080/#/c/16400/4/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@118 PS4, Line 118: final > We want a single instance per class, so this should be static. Then multiple instances of AsyncKuduSession will share the same throttler and manipulate the same copy of the lastLoggedTime variable, is that what we want? http://gerrit.cloudera.org:8080/#/c/16400/4/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@554 PS4, Line 554: 10 > I think 60 secs would be better. even if that means only log one message per 60 secs? http://gerrit.cloudera.org:8080/#/c/16400/4/java/kudu-client/src/main/java/org/apache/kudu/util/LogThrottler.java File java/kudu-client/src/main/java/org/apache/kudu/util/LogThrottler.java: http://gerrit.cloudera.org:8080/#/c/16400/4/java/kudu-client/src/main/java/org/apache/kudu/util/LogThrottler.java@42 PS4, Line 42: private long lastLoggedTime = 0; > Nit: If time were to start from epoch then the first log message will be th Done http://gerrit.cloudera.org:8080/#/c/16400/4/java/kudu-client/src/main/java/org/apache/kudu/util/LogThrottler.java@49 PS4, Line 49: * Calls shouldLog method, logs message at trace level if method true, does nothing if false > Java doc comments are meant for the users and there is no need to mention p Done -- To view, visit http://gerrit.cloudera.org:8080/16400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia2089b6fc905a5b54d664b7200060cabb965f40f Gerrit-Change-Number: 16400 Gerrit-PatchSet: 4 Gerrit-Owner: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Fri, 04 Sep 2020 00:18:42 +0000 Gerrit-HasComments: Yes
