Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16400 )
Change subject: KUDU-3012: Add a Log Throttler ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/16400/5/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/5/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@118 PS5, Line 118: /** > +1 Done 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: LogThrottler wil > nit: LogThrottler Done 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. > This is only really used in tests right now. An alternative would be to rem good point, took out the varargs support for now and put it in a TODO 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 lastLoggedTimeSecs = -1; > Do we need to protect concurrent updates to this? Done 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 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: 6 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 22:22:37 +0000 Gerrit-HasComments: Yes
