Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16400 )
Change subject: KUDU-3012: Add a Log Throttler ...................................................................... Patch Set 6: (1 comment) 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@34 PS5, Line 34: TODO(mreddy): Efficiently support variable arguments by explicitly checking for arguments length : * in logging functions and call corresponding function rather than calling function with varargs : * every time. This will avoid the hidden cost of creating an Object[] before invoking the method. > good point, took out the varargs support for now and put it in a TODO The only concern with variadic arguments is performance, right? Lets say another log message which we think should be throttled shows up soon and that takes arguments and not just message, wouldn't it be convenient to simply use this LogThrottle class instead of re-implementing stuff that Mahesh has already implemented? IMO it's better to keep the variadic argument support and tackle performance concerns once it really becomes one. -- 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: 6 Gerrit-Owner: Mahesh Reddy <mre...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@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: Thu, 10 Sep 2020 14:47:31 +0000 Gerrit-HasComments: Yes
