giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maja Kabiljo" <majakabi...@fb.com>
Subject Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Date Fri, 02 Nov 2012 19:28:42 GMT


> 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
> 
>


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