> On Nov. 2, 2012, 4:36 a.m., Alessandro Presta wrote: > > I'm not sure how I feel about using MasterCompute for something other than > > computation. I guess it gets the job done but I wonder if we should have a > > better place for this sort of code (like a MasterContext or whatever). It's > > just a matter of separating the roles. > > Other than that, the code looks good. > > Nitay Joffe wrote: > Actually I started it exactly that way but Maja said it's easiser / > cleaner to just add the method to MasterCompute rather than create a separate > MasterContext object. > > Alessandro Presta wrote: > The reason why this is problematic is that MasterCompute is supposed to > be used for defining an algorithm (together with Vertex). Say an application > uses MC to check some termination condition or working on aggregators. > If you also want to add stats the way we're doing it, you need to either > modify the same MC (mixing algorithm code with instrumentation) or make it > subclass a different MC. If you then want to plug that functionality on/off, > there is no seamless way. > I advocate for a clear separation of algorithm code and optional > logging/infrastructure code. > I agree it's slightly easier this way, but I wouldn't say it's cleaner.
We have Vertex computation per vertex and WorkerContext per worker, i.e. many vertices. On master you have just one master.compute, that's why I don't see the clear distinction between MasterCompute and MasterContext, it seems redundant having both. But if you feel it would add value, go ahead. When I suggested using master.compute for everything there were no arguments against it, yours sounds reasonable. - Maja ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7762/#review13024 ----------------------------------------------------------- On Nov. 2, 2012, 5:53 p.m., Nitay Joffe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7762/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2012, 5:53 p.m.) > > > Review request for giraph. > > > Description > ------- > > GIRAPH-390: MasterCompute postApplication callback. > > > Diffs > ----- > > giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java > 69751086ade95c4d645feafa5131587bb9f18206 > giraph/src/main/java/org/apache/giraph/counters/GiraphHadoopCounter.java > PRE-CREATION > giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java > PRE-CREATION > giraph/src/main/java/org/apache/giraph/counters/GiraphTimers.java > PRE-CREATION > giraph/src/main/java/org/apache/giraph/counters/HadoopCountersBase.java > PRE-CREATION > giraph/src/main/java/org/apache/giraph/counters/package-info.java > PRE-CREATION > giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java > 98a0bd2c6dc9602ea17612d4f68461d5fa43fb00 > giraph/src/main/java/org/apache/giraph/graph/MasterCompute.java > 9de23ad4a7781d6342339f26f44fb1c1d9bce2b0 > giraph/src/main/java/org/apache/giraph/graph/MasterThread.java > 85842d0ab59330d95b843c440f2d5705b41e0d1b > giraph/src/main/java/org/apache/giraph/metrics/EmptyMetricsRegistry.java > 262db29bbd4797e470f426999b85f2c1a7d1dee3 > giraph/src/main/java/org/apache/giraph/metrics/GiraphMetrics.java > ea1cc1ef69aaa2c786b99de51cd30e39ee9f1620 > giraph/src/main/java/org/apache/giraph/metrics/NoOpMetricsRegistry.java > PRE-CREATION > giraph/src/main/java/org/apache/giraph/metrics/package-info.java > f7529f52527e8e4253f579bd6d01865aa980926b > > Diff: https://reviews.apache.org/r/7762/diff/ > > > Testing > ------- > > > Thanks, > > Nitay Joffe > >
