Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16400 )
Change subject: KUDU-3012: Add a Log Throttler ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/16400/5/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/5/java/kudu-client/src/main/java/org/apache/kudu/util/LogThrottler.java@30 PS5, Line 30: ThrottlerLogUtil nit: LogThrottler http://gerrit.cloudera.org:8080/#/c/16400/5/java/kudu-client/src/main/java/org/apache/kudu/util/LogThrottler.java@34 PS5, Line 34: TODO(mreddy): If logging slows down drastically, explicitly check for arguments length in logging : * functions and call corresponding function rather than calling function with varargs each time. : * By calling the varargs function each time, even when it's not needed an Object[] will be created. This is only really used in tests right now. An alternative would be to remove the Object... arguments altogether, update the test to pass just the string, and update the TODO to efficiently support variadic arguments. http://gerrit.cloudera.org:8080/#/c/16400/5/java/kudu-client/src/main/java/org/apache/kudu/util/LogThrottler.java@43 PS5, Line 43: private long lastLoggedTime = -1; Do we need to protect concurrent updates to this? http://gerrit.cloudera.org:8080/#/c/16400/5/java/kudu-client/src/main/java/org/apache/kudu/util/LogThrottler.java@43 PS5, Line 43: lastLoggedTime nit: maybe lastLoggedTimeSecs so usage and units are more obviously correct elsewhere. -- 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: 5 Gerrit-Owner: Mahesh Reddy <mre...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <mre...@cloudera.com> Gerrit-Comment-Date: Wed, 09 Sep 2020 18:46:05 +0000 Gerrit-HasComments: Yes
