----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37069/#review94122 -----------------------------------------------------------
samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala (line 46) <https://reviews.apache.org/r/37069/#comment148638> Why are we adding a new clock here? changing clock() implementation to: val clock: () => Long = {System.nanoTime} Should work as well. samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala (line 49) <https://reviews.apache.org/r/37069/#comment148634> The variable should be lastWindowNs and lastCommitNs samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala (line 78) <https://reviews.apache.org/r/37069/#comment148635> This should be totalNs samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala (line 107) <https://reviews.apache.org/r/37069/#comment148636> This variable name should also be activeNs samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala (line 134) <https://reviews.apache.org/r/37069/#comment148637> Same here. samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala (line 135) <https://reviews.apache.org/r/37069/#comment148641> This does not need to be done by metricClocks. The original code works. Just that there is optimization we should do here to reduce the number of clock() calls. updateTimerAndGetDuration internally calls clock(), and we are calling clock() twice here as well. Should at least cache the return value from clock() and re-use it here. - Yi Pan (Data Infrastructure) On Aug. 4, 2015, 9:18 a.m., Aleksandar Pejakovic wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37069/ > ----------------------------------------------------------- > > (Updated Aug. 4, 2015, 9:18 a.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > Changed SystemProducersMetrics and RunLoop so that metrics now show > nanoseconds instead milliseconds. > > > Diffs > ----- > > samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala c292ae4 > > samza-core/src/main/scala/org/apache/samza/container/SamzaContainerMetrics.scala > aa7a9bc > > Diff: https://reviews.apache.org/r/37069/diff/ > > > Testing > ------- > > Tested on hello-samza -> wikipedia-parser, results: > ``` > "org.apache.samza.container.SamzaContainerMetrics":{ > "commit-calls":10, > "window-ns":3198.62544796632, > "process-null-envelopes":56292, > "process-envelopes":989, > "window-calls":0, > "commit-ns":5130.901534393375, > "send-calls":0, > "process-calls":57283, > "choose-ns":10368839.818551894, > "process-ns":10390588.194071393, > "event-loop-utilization":0.99807554 > } > ``` > > > Thanks, > > Aleksandar Pejakovic > >
