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/11/09 01:29:30 UTC
Review Request: GIRAPH-416: MasterObserver for user post-application
customization
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/
-----------------------------------------------------------
Review request for giraph.
Description
-------
GIRAPH-416: MasterObserver for user post-application customization
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/graph/BspServiceMaster.java 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
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/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/7981/diff/
Testing
-------
Thanks,
Nitay Joffe
Re: Review Request: GIRAPH-416: MasterObserver for user post-application
customization
Posted by Avery Ching <av...@gmail.com>.
> On Nov. 9, 2012, 5:52 a.m., Avery Ching wrote:
> > giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java, lines 437-458
> > <https://reviews.apache.org/r/7981/diff/2/?file=187558#file187558line437>
> >
> > These methods could be in GiraphConfiguration. Methods in ImmmutableClassesGiraphConfiguration should be for immutable variables (i.e. private final and assigned from the constructor) and for fast acesss.
>
> Nitay Joffe wrote:
> I can move getMasterObserverClasses() but for createMasterObservers() I need to pass in this so that it can get ImmmutableClassesGiraphConfigur-ed
Yeah, let's do that then.
- Avery
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/#review13284
-----------------------------------------------------------
On Nov. 9, 2012, 3:09 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7981/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 3:09 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-416: MasterObserver for user post-application customization
>
>
> Diffs
> -----
>
> giraph/src/main/java/org/apache/giraph/GiraphConfiguration.java 496bce7
> giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java bb6a739
> giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 688ce43
> giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb
> giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d
> giraph/src/main/java/org/apache/giraph/io/IntNullNullNullTextInputFormat.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/master/DefaultMasterObserver.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/utils/InternalVertexRunner.java b891690
> 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/7981/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-416: MasterObserver for user post-application
customization
Posted by Nitay Joffe <ni...@apache.org>.
> On Nov. 9, 2012, 5:52 a.m., Avery Ching wrote:
> > giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java, lines 437-458
> > <https://reviews.apache.org/r/7981/diff/2/?file=187558#file187558line437>
> >
> > These methods could be in GiraphConfiguration. Methods in ImmmutableClassesGiraphConfiguration should be for immutable variables (i.e. private final and assigned from the constructor) and for fast acesss.
I can move getMasterObserverClasses() but for createMasterObservers() I need to pass in this so that it can get ImmmutableClassesGiraphConfigur-ed
- Nitay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/#review13284
-----------------------------------------------------------
On Nov. 9, 2012, 3:09 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7981/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 3:09 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-416: MasterObserver for user post-application customization
>
>
> Diffs
> -----
>
> giraph/src/main/java/org/apache/giraph/GiraphConfiguration.java 496bce7
> giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java bb6a739
> giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 688ce43
> giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb
> giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d
> giraph/src/main/java/org/apache/giraph/io/IntNullNullNullTextInputFormat.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/master/DefaultMasterObserver.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/utils/InternalVertexRunner.java b891690
> 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/7981/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-416: MasterObserver for user post-application
customization
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/#review13284
-----------------------------------------------------------
This looks pretty good. One comment below though.
giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java
<https://reviews.apache.org/r/7981/#comment28544>
These methods could be in GiraphConfiguration. Methods in ImmmutableClassesGiraphConfiguration should be for immutable variables (i.e. private final and assigned from the constructor) and for fast acesss.
- Avery Ching
On Nov. 9, 2012, 3:09 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7981/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 3:09 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-416: MasterObserver for user post-application customization
>
>
> Diffs
> -----
>
> giraph/src/main/java/org/apache/giraph/GiraphConfiguration.java 496bce7
> giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java bb6a739
> giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 688ce43
> giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb
> giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d
> giraph/src/main/java/org/apache/giraph/io/IntNullNullNullTextInputFormat.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/master/DefaultMasterObserver.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/utils/InternalVertexRunner.java b891690
> 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/7981/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-416: MasterObserver for user post-application
customization
Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/#review13283
-----------------------------------------------------------
Ship it!
Thanks Nitay, +1.
- Maja Kabiljo
On Nov. 9, 2012, 3:09 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7981/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 3:09 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-416: MasterObserver for user post-application customization
>
>
> Diffs
> -----
>
> giraph/src/main/java/org/apache/giraph/GiraphConfiguration.java 496bce7
> giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java bb6a739
> giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 688ce43
> giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb
> giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d
> giraph/src/main/java/org/apache/giraph/io/IntNullNullNullTextInputFormat.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/master/DefaultMasterObserver.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/utils/InternalVertexRunner.java b891690
> 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/7981/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-416: MasterObserver for user post-application
customization
Posted by Nitay Joffe <ni...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/
-----------------------------------------------------------
(Updated Nov. 9, 2012, 3:09 a.m.)
Review request for giraph.
Changes
-------
maja's comments
Description
-------
GIRAPH-416: MasterObserver for user post-application customization
Diffs (updated)
-----
giraph/src/main/java/org/apache/giraph/GiraphConfiguration.java 496bce7
giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java bb6a739
giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 688ce43
giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb
giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d
giraph/src/main/java/org/apache/giraph/io/IntNullNullNullTextInputFormat.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/master/DefaultMasterObserver.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/utils/InternalVertexRunner.java b891690
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/7981/diff/
Testing
-------
Thanks,
Nitay Joffe
Re: Review Request: GIRAPH-416: MasterObserver for user post-application
customization
Posted by Nitay Joffe <ni...@apache.org>.
> On Nov. 9, 2012, 2:04 a.m., Maja Kabiljo wrote:
> > giraph/src/main/java/org/apache/giraph/master/MasterObserver.java, line 24
> > <https://reviews.apache.org/r/7981/diff/1/?file=187482#file187482line24>
> >
> > How about adding preApplication, pre/postSuperstep, like in WorkerContext? A
> > lso, I think MasterObserver should at least be Configurable, or have a GraphState in there, so we could access application state from implementing classes.
K I've made it ImmutableClassesGiraphConfigurable. If people need something bigger like GraphState we can change it later.
- Nitay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/#review13273
-----------------------------------------------------------
On Nov. 9, 2012, 12:29 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7981/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 12:29 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-416: MasterObserver for user post-application customization
>
>
> 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/graph/BspServiceMaster.java 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
> giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
> 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/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/7981/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-416: MasterObserver for user post-application
customization
Posted by Nitay Joffe <ni...@apache.org>.
> On Nov. 9, 2012, 2:04 a.m., Maja Kabiljo wrote:
> > giraph/src/main/java/org/apache/giraph/master/MasterObserver.java, line 24
> > <https://reviews.apache.org/r/7981/diff/1/?file=187482#file187482line24>
> >
> > How about adding preApplication, pre/postSuperstep, like in WorkerContext? A
> > lso, I think MasterObserver should at least be Configurable, or have a GraphState in there, so we could access application state from implementing classes.
>
> Nitay Joffe wrote:
> K I've made it ImmutableClassesGiraphConfigurable. If people need something bigger like GraphState we can change it later.
To be clear I chose this because GraphState has a bunch of Worker-specific things in it, so it really doesn't feel like the right thing to me. If anything maybe we need a separate MasterState or something.
- Nitay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/#review13273
-----------------------------------------------------------
On Nov. 9, 2012, 12:29 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7981/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 12:29 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-416: MasterObserver for user post-application customization
>
>
> 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/graph/BspServiceMaster.java 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
> giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
> 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/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/7981/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-416: MasterObserver for user post-application
customization
Posted by Nitay Joffe <ni...@apache.org>.
> On Nov. 9, 2012, 2:04 a.m., Maja Kabiljo wrote:
> > giraph/src/main/java/org/apache/giraph/master/MasterObserver.java, line 24
> > <https://reviews.apache.org/r/7981/diff/1/?file=187482#file187482line24>
> >
> > How about adding preApplication, pre/postSuperstep, like in WorkerContext? A
> > lso, I think MasterObserver should at least be Configurable, or have a GraphState in there, so we could access application state from implementing classes.
>
> Nitay Joffe wrote:
> K I've made it ImmutableClassesGiraphConfigurable. If people need something bigger like GraphState we can change it later.
>
> Nitay Joffe wrote:
> To be clear I chose this because GraphState has a bunch of Worker-specific things in it, so it really doesn't feel like the right thing to me. If anything maybe we need a separate MasterState or something.
>
> Alessandro Presta wrote:
> Regarding this, how about reorganizing functionality so that GraphState only holds global info on the graph, and all those implementation details go somewhere else?
> That way we can even hand it to user code (instead of having to wrap some of its methods (see Vertex).
agreed, let's open a JIRA and take it up in another task.
- Nitay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/#review13273
-----------------------------------------------------------
On Nov. 9, 2012, 12:29 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7981/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 12:29 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-416: MasterObserver for user post-application customization
>
>
> 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/graph/BspServiceMaster.java 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
> giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
> 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/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/7981/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-416: MasterObserver for user post-application
customization
Posted by Alessandro Presta <al...@fb.com>.
> On Nov. 9, 2012, 2:04 a.m., Maja Kabiljo wrote:
> > giraph/src/main/java/org/apache/giraph/master/MasterObserver.java, line 24
> > <https://reviews.apache.org/r/7981/diff/1/?file=187482#file187482line24>
> >
> > How about adding preApplication, pre/postSuperstep, like in WorkerContext? A
> > lso, I think MasterObserver should at least be Configurable, or have a GraphState in there, so we could access application state from implementing classes.
>
> Nitay Joffe wrote:
> K I've made it ImmutableClassesGiraphConfigurable. If people need something bigger like GraphState we can change it later.
>
> Nitay Joffe wrote:
> To be clear I chose this because GraphState has a bunch of Worker-specific things in it, so it really doesn't feel like the right thing to me. If anything maybe we need a separate MasterState or something.
Regarding this, how about reorganizing functionality so that GraphState only holds global info on the graph, and all those implementation details go somewhere else?
That way we can even hand it to user code (instead of having to wrap some of its methods (see Vertex).
- Alessandro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/#review13273
-----------------------------------------------------------
On Nov. 9, 2012, 12:29 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7981/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 12:29 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-416: MasterObserver for user post-application customization
>
>
> 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/graph/BspServiceMaster.java 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
> giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
> 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/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/7981/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-416: MasterObserver for user post-application
customization
Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7981/#review13273
-----------------------------------------------------------
Nice work! And thanks for splitting it up.
giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java
<https://reviews.apache.org/r/7981/#comment28531>
Formatting
giraph/src/main/java/org/apache/giraph/master/MasterObserver.java
<https://reviews.apache.org/r/7981/#comment28529>
How about adding preApplication, pre/postSuperstep, like in WorkerContext? A
lso, I think MasterObserver should at least be Configurable, or have a GraphState in there, so we could access application state from implementing classes.
- Maja Kabiljo
On Nov. 9, 2012, 12:29 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7981/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 12:29 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-416: MasterObserver for user post-application customization
>
>
> 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/graph/BspServiceMaster.java 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
> giraph/src/main/java/org/apache/giraph/graph/IntNullNullNullVertex.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
> 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/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/7981/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>