You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Nitay Joffe <ni...@apache.org> on 2012/10/29 17:05:07 UTC
Review Request: GIRAPH-390: MasterCompute postApplication callback.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7762/
-----------------------------------------------------------
Review request for giraph.
Description
-------
GIRAPH-390: MasterCompute postApplication callback.
Diffs
-----
giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java d68debc41b44e20c0dcb5907d1a1b398b759b149
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/GiraphHadoopCounter.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/GiraphStats.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/GiraphTimers.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/HadoopCountersBase.java PRE-CREATION
Diff: https://reviews.apache.org/r/7762/diff/
Testing
-------
Thanks,
Nitay Joffe
Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Posted by Nitay Joffe <ni...@apache.org>.
> 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.
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.
> On Nov. 2, 2012, 4:36 a.m., Alessandro Presta wrote:
> > giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java, line 178
> > <https://reviews.apache.org/r/7762/diff/3/?file=183499#file183499line178>
> >
> > Mmh, isn't there anything more compact than this?
> > I guess you could keep the counters in an array/ArrayList, and have constants like SUPERSTEP = 0, VERTICES = 1.
> > That way you don't even need the inner class, just build an Iterator from the array on the fly.
Did same in GiraphTimers too.
- Nitay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7762/#review13024
-----------------------------------------------------------
On Oct. 31, 2012, 7:43 p.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7762/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2012, 7:43 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-390: MasterCompute postApplication callback.
>
>
> Diffs
> -----
>
> 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
>
>
Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Posted by Maja Kabiljo <ma...@fb.com>.
> 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
>
>
Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Posted by Alessandro Presta <al...@fb.com>.
> 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.
>
> Maja Kabiljo wrote:
> 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.
See, my point is also that WorkerContext shouldn't be used for computation.
Workers are an implementation detail of Giraph. The user writing an algorithm shouldn't bother with workers at all. I think we already discussed this when revamping aggregators.
Now when one wants to monitor the infrastructure, like in Nitay's case, it makes sense to work at infrastructure level, i.e. mess with the WorkerContext.
Although, we're now discussing a solution based on the observer pattern, where one can register multiple observers with their callbacks as opposed to sticking everything in a single class.
That is, you have only one Vertex and only one MasterCompute (to define the algorithm), but you can register any number of MasterObserver and WorkerObserver (to run arbitrary code at specific times).
How does this sound?
- Alessandro
-----------------------------------------------------------
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
>
>
Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Posted by Alessandro Presta <al...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7762/#review13024
-----------------------------------------------------------
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.
giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java
<https://reviews.apache.org/r/7762/#comment28034>
I think we use the form "vertices" everywhere else in the codebase. Both are considered correct though.
giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java
<https://reviews.apache.org/r/7762/#comment28035>
Mmh, isn't there anything more compact than this?
I guess you could keep the counters in an array/ArrayList, and have constants like SUPERSTEP = 0, VERTICES = 1.
That way you don't even need the inner class, just build an Iterator from the array on the fly.
- Alessandro Presta
On Oct. 31, 2012, 7:43 p.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7762/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2012, 7:43 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-390: MasterCompute postApplication callback.
>
>
> Diffs
> -----
>
> 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
>
>
Re: Review Request: GIRAPH-390: Observer for end of computation on Master
Posted by Nitay Joffe <ni...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7762/
-----------------------------------------------------------
(Updated Nov. 7, 2012, 7:22 p.m.)
Review request for giraph.
Summary (updated)
-----------------
GIRAPH-390: Observer for end of computation on Master
Description (updated)
-------
https://issues.apache.org/jira/browse/GIRAPH-390
Diffs
-----
giraph/src/main/java/org/apache/giraph/GiraphConfiguration.java 496bce7e13cd282337d8bfd8526a25657316ea33
giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java bb6a739749d6110faf0a39f1b3bfee769bea28a2
giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 688ce4389a9b5ab6ed3d124af92936286563f21d
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 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
giraph/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 8801dde5102ffca397678b0dcc3dd7c2932d22d7
giraph/src/main/java/org/apache/giraph/graph/HashMapVertex.java 26b14caed23706b12f21142862bbd15fe3720bd6
giraph/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java a23b87f7e43d6a3b78516cbb5f1dd512dca84a8d
giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1db6e977eb4a8102351a908dc1a113935002e61e
giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
giraph/src/main/java/org/apache/giraph/graph/SimpleMutableVertex.java 85af9700e76a3ef32191fe0dc5631f430b13de42
giraph/src/main/java/org/apache/giraph/graph/SimpleVertex.java ed6759a884553fa15ee477e19e483e1d210ba7be
giraph/src/main/java/org/apache/giraph/graph/Vertex.java 8789ee5c8f170b6d598f0b36053333b1f6f36227
giraph/src/main/java/org/apache/giraph/io/IntNullNullNullTextInputFormat.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/master/MasterObserver.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/master/package-info.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/EmptyMetricsRegistry.java 262db29bbd4797e470f426999b85f2c1a7d1dee3
giraph/src/main/java/org/apache/giraph/metrics/GiraphMetricsRegistry.java eac9f7295d968c3b30c40b89944e8f9f8549cc70
giraph/src/main/java/org/apache/giraph/metrics/NoOpMetricsRegistry.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/package-info.java f7529f52527e8e4253f579bd6d01865aa980926b
giraph/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java b891690474ba2b64ed2778c332a7855565a64e5b
giraph/src/test/java/org/apache/giraph/TestGiraphConfiguration.java PRE-CREATION
giraph/src/test/java/org/apache/giraph/TestMasterObserver.java PRE-CREATION
Diff: https://reviews.apache.org/r/7762/diff/
Testing
-------
Thanks,
Nitay Joffe
Re: Review Request: GIRAPH-390: MasterCompute end of application callback
Posted by Nitay Joffe <ni...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7762/
-----------------------------------------------------------
(Updated Nov. 7, 2012, 7:16 p.m.)
Review request for giraph.
Changes
-------
Rebased to latest trunk.
Using new MasterObserver instead of MasterCompute now. Note I added some helpful functions to GiraphConfiguration for handling lists of classes. This should come in handy in the future as we do more Observer work.
Summary (updated)
-----------------
GIRAPH-390: MasterCompute end of application callback
Description (updated)
-------
GIRAPH-390: MasterCompute end of application callback
Diffs (updated)
-----
giraph/src/main/java/org/apache/giraph/GiraphConfiguration.java 496bce7e13cd282337d8bfd8526a25657316ea33
giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java bb6a739749d6110faf0a39f1b3bfee769bea28a2
giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 688ce4389a9b5ab6ed3d124af92936286563f21d
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 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
giraph/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 8801dde5102ffca397678b0dcc3dd7c2932d22d7
giraph/src/main/java/org/apache/giraph/graph/HashMapVertex.java 26b14caed23706b12f21142862bbd15fe3720bd6
giraph/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java a23b87f7e43d6a3b78516cbb5f1dd512dca84a8d
giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1db6e977eb4a8102351a908dc1a113935002e61e
giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
giraph/src/main/java/org/apache/giraph/graph/SimpleMutableVertex.java 85af9700e76a3ef32191fe0dc5631f430b13de42
giraph/src/main/java/org/apache/giraph/graph/SimpleVertex.java ed6759a884553fa15ee477e19e483e1d210ba7be
giraph/src/main/java/org/apache/giraph/graph/Vertex.java 8789ee5c8f170b6d598f0b36053333b1f6f36227
giraph/src/main/java/org/apache/giraph/io/IntNullNullNullTextInputFormat.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/master/MasterObserver.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/master/package-info.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/EmptyMetricsRegistry.java 262db29bbd4797e470f426999b85f2c1a7d1dee3
giraph/src/main/java/org/apache/giraph/metrics/GiraphMetricsRegistry.java eac9f7295d968c3b30c40b89944e8f9f8549cc70
giraph/src/main/java/org/apache/giraph/metrics/NoOpMetricsRegistry.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/package-info.java f7529f52527e8e4253f579bd6d01865aa980926b
giraph/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java b891690474ba2b64ed2778c332a7855565a64e5b
giraph/src/test/java/org/apache/giraph/TestGiraphConfiguration.java PRE-CREATION
giraph/src/test/java/org/apache/giraph/TestMasterObserver.java PRE-CREATION
Diff: https://reviews.apache.org/r/7762/diff/
Testing
-------
Thanks,
Nitay Joffe
Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7762/#review13053
-----------------------------------------------------------
giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java
<https://reviews.apache.org/r/7762/#comment28092>
Thanks for cleaning this up.
- Avery Ching
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
>
>
Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Posted by Nitay Joffe <ni...@apache.org>.
-----------------------------------------------------------
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.
Changes
-------
Alessandro and Maja's comments. Created postApplication() in BSM instead of using cleanup().
Description
-------
GIRAPH-390: MasterCompute postApplication callback.
Diffs (updated)
-----
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
Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Posted by Nitay Joffe <ni...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7762/
-----------------------------------------------------------
(Updated Oct. 31, 2012, 7:43 p.m.)
Review request for giraph.
Changes
-------
I moved the counters to package org.apache.giraph.counters so that it's separate from the yammer metrics stuff we have. Otherwise the same patch.
Description
-------
GIRAPH-390: MasterCompute postApplication callback.
Diffs (updated)
-----
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
Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Posted by Nitay Joffe <ni...@apache.org>.
> On Oct. 29, 2012, 9:44 p.m., Maja Kabiljo wrote:
> > Thanks Nitay, this is a nice feature to have.
> >
> > I like that you pulled out counters to separate classes. But why do you use static instances of these classes, can't you have them as fields of BspServiceMaster/MasterThread?
> >
> > Also, it would be nicer if postApplication wasn't called from inside cleanup.
Just to make it easier to access from everywhere, e.g. from inside a MasterCompute. I didn't see them being anything other than singletons?
Where should I stick postApplication?
- Nitay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7762/#review12888
-----------------------------------------------------------
On Oct. 29, 2012, 5:20 p.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7762/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2012, 5:20 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-390: MasterCompute postApplication callback.
>
>
> Diffs
> -----
>
> giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java d68debc41b44e20c0dcb5907d1a1b398b759b149
> 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/GiraphHadoopCounter.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/GiraphStats.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/GiraphTimers.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/HadoopCountersBase.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/package-info.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7762/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7762/#review12888
-----------------------------------------------------------
Thanks Nitay, this is a nice feature to have.
I like that you pulled out counters to separate classes. But why do you use static instances of these classes, can't you have them as fields of BspServiceMaster/MasterThread?
Also, it would be nicer if postApplication wasn't called from inside cleanup.
- Maja Kabiljo
On Oct. 29, 2012, 5:20 p.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7762/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2012, 5:20 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-390: MasterCompute postApplication callback.
>
>
> Diffs
> -----
>
> giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java d68debc41b44e20c0dcb5907d1a1b398b759b149
> 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/GiraphHadoopCounter.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/GiraphStats.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/GiraphTimers.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/HadoopCountersBase.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/package-info.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7762/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-390: MasterCompute postApplication callback.
Posted by Nitay Joffe <ni...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7762/
-----------------------------------------------------------
(Updated Oct. 29, 2012, 5:20 p.m.)
Review request for giraph.
Changes
-------
Forgot to run checkstyle whatnots. This version passes 'mvn clean install' for me
Description
-------
GIRAPH-390: MasterCompute postApplication callback.
Diffs (updated)
-----
giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java d68debc41b44e20c0dcb5907d1a1b398b759b149
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/GiraphHadoopCounter.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/GiraphStats.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/GiraphTimers.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/HadoopCountersBase.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/package-info.java PRE-CREATION
Diff: https://reviews.apache.org/r/7762/diff/
Testing
-------
Thanks,
Nitay Joffe