giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eli Reisman <apache.mail...@gmail.com>
Subject Re: Review Request: GIRAPH-232: Add metrics system into Giraph
Date Tue, 16 Oct 2012 04:04:16 GMT
Sounds great!

On Mon, Oct 15, 2012 at 1:50 PM, Nitay Joffe <nitay@apache.org> wrote:

>
>
> > On Oct. 15, 2012, 5:59 p.m., Eli Reisman wrote:
> > > Hi Nitay,
> > >
> > > Great job, I got to use this patch for a time before Jakob had to set
> it aside and it is incredibly helpful. A couple questions/concerns:
> > >
> > > Be careful about hardcoding any metrics objects into the codebase,
> usually the calls to the metrics package can delay processing and given a
> job of sufficient size, even periodic messaging from a bunch of worker
> tasks to the graphite server picking up the metrics data will overwhelm
> that server and it will lose data or connections. Your default of 90
> seconds per metrics report is good but even then I found when debugging or
> analyzing runs for best results we had to add metrics objects to a build of
> trunk, then remove again once we had the data we needed.
> > >
> > > Also, I'm a bit fuzzy on the implementation of the EmptyRegistry and
> metrics -- is each singleton instance (like the EmptyCounter) simply
> managing all Counter objects for that JVM/class loader's metric instances
> and updating all of them by name parameter? How are we using a singleton
> here?
> > >
> > > You might want to pair this with a quick wiki page on how to set up
> the receiving metrics server and/or configure and add metrics objects to
> the Giraph source for those who want to use this feature. I think if people
> adopt this it will really accelerate development and testing for Giraph
> contributors accross the board.
> > >
> > > Again, thanks so much for doing this, I was considering suggesting
> taking over from Jakob myself but I figured he'd get around to this when he
> had time.
>
> There is no metric server / graphite piece. I actually ripped that piece
> out (but can put it back in). The every 90 seconds is printing to console,
> which is completely configurable (can turn it off all together).
> The Empty* classes are in case you wish to turn off metrics all together.
> They are dummy classes so that the user code stays the same while having no
> effect.
> Does that make sense?
>
>
> - Nitay
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7595/#review12444
> -----------------------------------------------------------
>
>
> On Oct. 15, 2012, 7:18 a.m., Nitay Joffe wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/7595/
> > -----------------------------------------------------------
> >
> > (Updated Oct. 15, 2012, 7:18 a.m.)
> >
> >
> > Review request for giraph.
> >
> >
> > Description
> > -------
> >
> > GIRAPH-232: Add metrics system into Giraph
> >
> >
> > Diffs
> > -----
> >
> >   giraph/pom.xml 31da7b45d90104459be07eefe28d59f71fd42c3b
> >   giraph/src/main/java/com/yammer/metrics/core/EmptyCounter.java
> PRE-CREATION
> >   giraph/src/main/java/com/yammer/metrics/core/EmptyExecutorService.java
> PRE-CREATION
> >   giraph/src/main/java/com/yammer/metrics/core/EmptyHistogram.java
> PRE-CREATION
> >   giraph/src/main/java/com/yammer/metrics/core/EmptyMeter.java
> PRE-CREATION
> >   giraph/src/main/java/com/yammer/metrics/core/EmptyTimer.java
> PRE-CREATION
> >   giraph/src/main/java/com/yammer/metrics/core/NullGuage.java
> PRE-CREATION
> >   giraph/src/main/java/com/yammer/metrics/core/package-info.java
> PRE-CREATION
> >   giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java
> efef8f69de4ff39e508e6e2141e1e14bfff70c37
> >   giraph/src/main/java/org/apache/giraph/comm/netty/ByteCounter.java
> f228375a7398a6c38134a38bf2055a17a998a204
> >   giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java
> 9e541f3a37fe71174d42aa2ee905d2a9e6b8a65c
> >
> giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java
> 6ea20bc8b2d2c0f9d727406eff88ce35cbc139f0
> >
> giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientServer.java
> 6600e78c2856befe5e67bf843a7028b493ede694
> >   giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java
> ade33a1d7320e9ed381f1fe62213de7d4ce8c3a1
> >   giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
> ff59f40eb3b7e7008bf578a57be22e8996540f8b
> >   giraph/src/main/java/org/apache/giraph/graph/GraphMapper.java
> e9217f7689b72008075805e7f504e00bf06f7df1
> >   giraph/src/main/java/org/apache/giraph/graph/Vertex.java
> cd78810344fdb3fdf29f5c471efb97c4de75c818
> >
> giraph/src/main/java/org/apache/giraph/metrics/EmptyMetricsRegistry.java
> PRE-CREATION
> >   giraph/src/main/java/org/apache/giraph/metrics/GiraphMetrics.java
> PRE-CREATION
> >   giraph/src/main/java/org/apache/giraph/metrics/package-info.java
> PRE-CREATION
> >   giraph/src/main/java/org/apache/giraph/utils/GiraphTimer.java
> PRE-CREATION
> >   giraph/src/main/java/org/apache/giraph/utils/MemoryUtils.java
> 8fc403201b784bd4dab733d6764d1cf7ed0295a6
> >   giraph/src/main/java/org/apache/giraph/utils/MilliSecTimer.java
> PRE-CREATION
> >   pom.xml 1fd2791bf42df465f532fbdacfd1979e6b4b3857
> >
> > Diff: https://reviews.apache.org/r/7595/diff/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Nitay Joffe
> >
> >
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message