Giraph
  1. Giraph
  2. GIRAPH-47

Export Worker's Context/State to vertices through pre/post/Application/Superstep

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.1.0
    • Fix Version/s: 0.1.0
    • Component/s: graph
    • Labels:
      None

      Description

      It would be quite useful for vertices to reach some worker-related information stored i.e. in the GraphState class.

      This information could be exported as a parameter to pre/post/Application/Superstep like this:

      public void preApplication(Configurable workerObject);
      public void postApplication(Configurable workerObject);
      public void preSuperstep(Configurable workerObject);
      public void postSuperstep(Configurable workerObject);
      public Configurable getWorkerObject();

      Another possibility is to add a Context inner class to BasicVertex to store this information.

      1. GIRAPH-47.final
        59 kB
        Avery Ching
      2. GIRAPH-47.diff
        54 kB
        Claudio Martella
      3. GIRAPH-47.diff
        51 kB
        Claudio Martella
      4. GIRAPH-47.diff
        17 kB
        Claudio Martella

        Activity

        Hide
        Claudio Martella added a comment -

        I'm very much in need for this feature. I'm willing to implement this with a little guidance (mostly a decision about what to store into this object).

        What I'd do would be one of the two:

        a) pass directly Mapper.Context (greedy approach, that's exactly what I need)
        b) our own Context class to be defined. In that case we should discuss the interface.

        Show
        Claudio Martella added a comment - I'm very much in need for this feature. I'm willing to implement this with a little guidance (mostly a decision about what to store into this object). What I'd do would be one of the two: a) pass directly Mapper.Context (greedy approach, that's exactly what I need) b) our own Context class to be defined. In that case we should discuss the interface.
        Hide
        Claudio Martella added a comment -

        another much easier way, at least for me, would be just to add a getContext() inside of BasicVertex and return GraphMapper's Context. In this case I'd guess we could also change the definition of Mapper from <Object, Object, Object, Object>, although it should work anyway if the the object to write implements toString() accordingly.

        Or, third possibility, adding an emit(Writable key, Writable value) method to BasicVertex that forwards to Context.write().

        Show
        Claudio Martella added a comment - another much easier way, at least for me, would be just to add a getContext() inside of BasicVertex and return GraphMapper's Context. In this case I'd guess we could also change the definition of Mapper from <Object, Object, Object, Object>, although it should work anyway if the the object to write implements toString() accordingly. Or, third possibility, adding an emit(Writable key, Writable value) method to BasicVertex that forwards to Context.write().
        Hide
        Claudio Martella added a comment -

        another much easier way, at least for me, would be just to add a getContext() inside of BasicVertex and return GraphMapper's Context. In this case I'd guess we could also change the definition of Mapper from <Object, Object, Object, Object>, although it should work anyway if the the object to write implements toString() accordingly.

        Or, third possibility, adding an emit(Writable key, Writable value) method to BasicVertex that forwards to Context.write().

        Show
        Claudio Martella added a comment - another much easier way, at least for me, would be just to add a getContext() inside of BasicVertex and return GraphMapper's Context. In this case I'd guess we could also change the definition of Mapper from <Object, Object, Object, Object>, although it should work anyway if the the object to write implements toString() accordingly. Or, third possibility, adding an emit(Writable key, Writable value) method to BasicVertex that forwards to Context.write().
        Hide
        Avery Ching added a comment -

        I tried to add you as a contributor to assign this to you, but I think JIRA has recently changed how that's done (by search) and that failed. I'll try again later, sorry.

        Unfortunately, I doubt that giving Mapper.Context to users will do what you want. We purposely use the output format in the job to call the user's vertex output format. Plus there is the issue of types that you brought up.

        I think it makes more sense to define our own worker context interface for these reasons.

        In fact, maybe it makes more sense to do away with the methods

        public void preApplication(Configurable workerObject);
        public void postApplication(Configurable workerObject);
        public void preSuperstep(Configurable workerObject);
        public void postSuperstep(Configurable workerObject);

        and instead put them on the WorkerContext (don't know what else to call it, you can rename it to something better). This will make it more clear about the fact that worker context is on a per-worker basis and the methods called with respect to a worker and not a vertex.

        I.e.

        public interface WorkerContext

        { void initialize(); // Called before anything else void close(); // Called when worker is done void preSuperstep(); // Called before compute() on any vertex void postSuperstep(); // Called after all vertices computed() }

        and then the user can register their own implementation. You can implement opening and closing files in initialize/close respectively. and you can dump whenever you want in preSuperstep/postSuperstep. By the way, please change the method names if you can think of something better. =)

        Show
        Avery Ching added a comment - I tried to add you as a contributor to assign this to you, but I think JIRA has recently changed how that's done (by search) and that failed. I'll try again later, sorry. Unfortunately, I doubt that giving Mapper.Context to users will do what you want. We purposely use the output format in the job to call the user's vertex output format. Plus there is the issue of types that you brought up. I think it makes more sense to define our own worker context interface for these reasons. In fact, maybe it makes more sense to do away with the methods public void preApplication(Configurable workerObject); public void postApplication(Configurable workerObject); public void preSuperstep(Configurable workerObject); public void postSuperstep(Configurable workerObject); and instead put them on the WorkerContext (don't know what else to call it, you can rename it to something better). This will make it more clear about the fact that worker context is on a per-worker basis and the methods called with respect to a worker and not a vertex. I.e. public interface WorkerContext { void initialize(); // Called before anything else void close(); // Called when worker is done void preSuperstep(); // Called before compute() on any vertex void postSuperstep(); // Called after all vertices computed() } and then the user can register their own implementation. You can implement opening and closing files in initialize/close respectively. and you can dump whenever you want in preSuperstep/postSuperstep. By the way, please change the method names if you can think of something better. =)
        Hide
        Claudio Martella added a comment -

        Sounds very reasonable. We would then remove those methods from BasicVertex API I guess. I'll work on this. Unfortunately I can't test it on my application as it's blocked by GIRAPH-36, so I'll think about a toy-example to test this out.

        One question: what would be this Configurable workerObject you'd pass to the WorkerContext implementation?

        Show
        Claudio Martella added a comment - Sounds very reasonable. We would then remove those methods from BasicVertex API I guess. I'll work on this. Unfortunately I can't test it on my application as it's blocked by GIRAPH-36 , so I'll think about a toy-example to test this out. One question: what would be this Configurable workerObject you'd pass to the WorkerContext implementation?
        Hide
        Avery Ching added a comment -

        Sorry if I wasn't clear, but we could do away with the Configurable workerObject and instead have a method in BasicVertex:

        WorkerContext getWorkerContext();

        While you may be blocked by GIRAPH-36 for your test, you can still try this out on the unittests as they do use the pre/post/application/superstep methods (i.e. SimplePageRankVertex). I've modified the interface slightly to give the Context at the beginning.

        public interface WorkerContext {
            void initialize(Context context); // Called before anything done on this worker                                          
            void close(); // Called when worker is done                           
            void preSuperstep(); // Called before compute() on any vertex                              
            void postSuperstep(); // Called after all vertices have run compute()                           
        }
        
        Show
        Avery Ching added a comment - Sorry if I wasn't clear, but we could do away with the Configurable workerObject and instead have a method in BasicVertex: WorkerContext getWorkerContext(); While you may be blocked by GIRAPH-36 for your test, you can still try this out on the unittests as they do use the pre/post/application/superstep methods (i.e. SimplePageRankVertex). I've modified the interface slightly to give the Context at the beginning. public interface WorkerContext { void initialize(Context context); // Called before anything done on this worker void close(); // Called when worker is done void preSuperstep(); // Called before compute() on any vertex void postSuperstep(); // Called after all vertices have run compute() }
        Hide
        Claudio Martella added a comment -

        I've given a thought or two to this interface and I cannot really see an easy way to pass information to the WorkerContext, a part of the initialize(). For example, for my particular Emitter scenario, how would i pass at preSuperstep() or postSuperstep() data to write to the HDFS or in general some parameters that might be needed?

        One way would be to create at Vertex.preApplication() a shared List, where I'd put the data that the WorkerContext would then dump to disk. It's not the best way as it could potentially eat a lot of memory.

        Or, I could add an emit() method to the implementing class and call it directly in compute(), like: ((MyEmitterWorkerContext)this.getWorkerContext()).emit(mydata). The engineering should be able to handle this quite easily (except for the cast cost each time, which is something).

        Any idea?

        Show
        Claudio Martella added a comment - I've given a thought or two to this interface and I cannot really see an easy way to pass information to the WorkerContext, a part of the initialize(). For example, for my particular Emitter scenario, how would i pass at preSuperstep() or postSuperstep() data to write to the HDFS or in general some parameters that might be needed? One way would be to create at Vertex.preApplication() a shared List, where I'd put the data that the WorkerContext would then dump to disk. It's not the best way as it could potentially eat a lot of memory. Or, I could add an emit() method to the implementing class and call it directly in compute(), like: ((MyEmitterWorkerContext)this.getWorkerContext()).emit(mydata). The engineering should be able to handle this quite easily (except for the cast cost each time, which is something). Any idea?
        Hide
        Avery Ching added a comment -

        I was thinking about the later approach that you mentioned ((MyEmitterWorkerContext)this.getWorkerContext()).emit(mydata). I doubt the cast is that expensive, it's not boxing/unboxing. If there is a cleaner way to do it that would allow users to do pretty much anything with their WorkerContext, I'm open to it. But this should work.

        Show
        Avery Ching added a comment - I was thinking about the later approach that you mentioned ((MyEmitterWorkerContext)this.getWorkerContext()).emit(mydata). I doubt the cast is that expensive, it's not boxing/unboxing. If there is a cleaner way to do it that would allow users to do pretty much anything with their WorkerContext, I'm open to it. But this should work.
        Hide
        Claudio Martella added a comment -

        This basically implements the interface discussed. It also add a nothing-doing WorkerContext (called DumpWorkerContext) and a SimpleVertexWithWorkerContext example. At this point BasicVertex.pre/post/Application/Superstep API could be removed and the user might just use this one. Also, checkpointing of WorkerContext could be discussed.

        Show
        Claudio Martella added a comment - This basically implements the interface discussed. It also add a nothing-doing WorkerContext (called DumpWorkerContext) and a SimpleVertexWithWorkerContext example. At this point BasicVertex.pre/post/Application/Superstep API could be removed and the user might just use this one. Also, checkpointing of WorkerContext could be discussed.
        Hide
        Claudio Martella added a comment -

        This basically implements the interface discussed. It also add a nothing-doing WorkerContext (called DumpWorkerContext) and a SimpleVertexWithWorkerContext example. At this point BasicVertex.pre/post/Application/Superstep API could be removed and the user might just use this one. Also, checkpointing of WorkerContext could be discussed.

        Show
        Claudio Martella added a comment - This basically implements the interface discussed. It also add a nothing-doing WorkerContext (called DumpWorkerContext) and a SimpleVertexWithWorkerContext example. At this point BasicVertex.pre/post/Application/Superstep API could be removed and the user might just use this one. Also, checkpointing of WorkerContext could be discussed.
        Hide
        Avery Ching added a comment -

        Claudio, great work! Couple of thoughts.

        1) Probably rename DumbWorkerContext to something like a bit more PC, i.e. UnusedWorkerContext.

        2) Could you remove the pre/post/Application/Superstep() code so we only have your way available? If you start by removing the

        public void preApplication()
        public void postApplication()
        public void preSuperstep()
        public void postSuperstep()

        methods from class Vertex, that should pretty much force you to fix everything else. Then once it passes unittests, please resubmit.

        3) Checkpointing is an interesting idea, but my guess is out of the scope of this issue. In the case of emitting to HDFS, I don't think it'll do what you expect since HDFS files are immutable.

        Show
        Avery Ching added a comment - Claudio, great work! Couple of thoughts. 1) Probably rename DumbWorkerContext to something like a bit more PC, i.e. UnusedWorkerContext. 2) Could you remove the pre/post/Application/Superstep() code so we only have your way available? If you start by removing the public void preApplication() public void postApplication() public void preSuperstep() public void postSuperstep() methods from class Vertex, that should pretty much force you to fix everything else. Then once it passes unittests, please resubmit. 3) Checkpointing is an interesting idea, but my guess is out of the scope of this issue. In the case of emitting to HDFS, I don't think it'll do what you expect since HDFS files are immutable.
        Hide
        Claudio Martella added a comment -

        3) Yes, in my case i just create a file per attempt, I'll have to de-duplicate later. I was thinking altruistically at other possible users

        Ok, will do!

        Show
        Claudio Martella added a comment - 3) Yes, in my case i just create a file per attempt, I'll have to de-duplicate later. I was thinking altruistically at other possible users Ok, will do!
        Hide
        Claudio Martella added a comment -

        Ok, I'm syncing with trunk and removing the previous pre/post/Superstep/Application.

        Two questions:

        • do we still want WorkerContext.preApplication() to throw InstantiationException and IllegalAccessException
        • does it still make sense to keep the RepresentativeVertex to just pass the graphState at each superstep?
        Show
        Claudio Martella added a comment - Ok, I'm syncing with trunk and removing the previous pre/post/Superstep/Application. Two questions: do we still want WorkerContext.preApplication() to throw InstantiationException and IllegalAccessException does it still make sense to keep the RepresentativeVertex to just pass the graphState at each superstep?
        Hide
        Claudio Martella added a comment -

        Fixing the examples and benchmarks to the new API.

        Problem:

        How do vertices register aggregators? WorkerContext accesses only Mapper.Context. I propose to change the API and pass GraphState at preApplication() instead. This way the WorkerContext can also have a broader view about the current state of graph and computation.

        Any cons?

        Show
        Claudio Martella added a comment - Fixing the examples and benchmarks to the new API. Problem: How do vertices register aggregators? WorkerContext accesses only Mapper.Context. I propose to change the API and pass GraphState at preApplication() instead. This way the WorkerContext can also have a broader view about the current state of graph and computation. Any cons?
        Hide
        Avery Ching added a comment -

        do we still want WorkerContext.preApplication() to throw InstantiationException and IllegalAccessException

        No, its probably not necessary. I think it was a quick fix to do aggreagator instantiation.

        does it still make sense to keep the RepresentativeVertex to just pass the graphState at each superstep?

        Representative vertex is not necessary anymore. This is a big improvement I think.

        I'm not a fan of exposing GraphState to vertex developers. You should be able to register aggregators by letting the WorkerContext implement the Aggregator methods as a dispatcher. What do you think?

        Show
        Avery Ching added a comment - do we still want WorkerContext.preApplication() to throw InstantiationException and IllegalAccessException No, its probably not necessary. I think it was a quick fix to do aggreagator instantiation. does it still make sense to keep the RepresentativeVertex to just pass the graphState at each superstep? Representative vertex is not necessary anymore. This is a big improvement I think. I'm not a fan of exposing GraphState to vertex developers. You should be able to register aggregators by letting the WorkerContext implement the Aggregator methods as a dispatcher. What do you think?
        Hide
        Claudio Martella added a comment -

        I'm actually kind of a big fan of passing the GraphState to WorkerContext. I've already changed the API accordingly and noticed it's often very handy. Checkout what pre/post/superstep/application code does in the examples and benchmarks, it often relies on things like the current superstep, the number of vertices etc, all things that are easily accessible by GraphState. Maybe we might want to hide some of them by making WorkerContext an abstract and proxy them? In this case we might want to proxy the aggregatorUsage code as well. Wouldn't this go more towards the direction Jakob was thinking? All this could then be simply removed from BasicVertex.

        Currently WorkerContext cannot implement Aggregator methods as it doesn't have a handle to AggregatorUsage, or am I missing something?

        Show
        Claudio Martella added a comment - I'm actually kind of a big fan of passing the GraphState to WorkerContext. I've already changed the API accordingly and noticed it's often very handy. Checkout what pre/post/superstep/application code does in the examples and benchmarks, it often relies on things like the current superstep, the number of vertices etc, all things that are easily accessible by GraphState. Maybe we might want to hide some of them by making WorkerContext an abstract and proxy them? In this case we might want to proxy the aggregatorUsage code as well. Wouldn't this go more towards the direction Jakob was thinking? All this could then be simply removed from BasicVertex. Currently WorkerContext cannot implement Aggregator methods as it doesn't have a handle to AggregatorUsage, or am I missing something?
        Hide
        Avery Ching added a comment -

        I agree with the proxy approach of WorkerContext so that a user can access a subset of the graph state methods without directly accessing graph state. You don't want the user to for instance "setGraphMapper".

        Show
        Avery Ching added a comment - I agree with the proxy approach of WorkerContext so that a user can access a subset of the graph state methods without directly accessing graph state. You don't want the user to for instance "setGraphMapper".
        Hide
        Claudio Martella added a comment -

        Implemented WorkerContext as abstract class proxing graphState. Removed pre/post/Application/Superstep from BasicVertex API.

        passes tests.

        Show
        Claudio Martella added a comment - Implemented WorkerContext as abstract class proxing graphState. Removed pre/post/Application/Superstep from BasicVertex API. passes tests.
        Hide
        Avery Ching added a comment -

        Claudio,

        Just checked out your diff. Overall looks pretty good, cleans up the code nicely. However, I am having the same trouble building it that I mentioned in GIRAPH-64. You don't have this issue?

        mvn package
        [INFO] Scanning for projects...
        [INFO]
        [INFO] ------------------------------------------------------------------------
        [INFO] Building Apache Incubator Giraph 0.70
        [INFO] ------------------------------------------------------------------------
        [INFO]
        [INFO] — maven-enforcer-plugin:1.0.1:enforce (enforce-maven) @ giraph —
        [INFO]
        [INFO] — maven-resources-plugin:2.4.3:resources (default-resources) @ giraph —
        [INFO] Using 'UTF-8' encoding to copy filtered resources.
        [INFO] skip non existing resourceDirectory /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/resources
        [INFO]
        [INFO] — maven-compiler-plugin:2.3.2:compile (default-compile) @ giraph —
        [INFO] Compiling 90 source files to /Users/aching/Documents/workspace/Trunk Apache Giraph/target/classes
        [INFO] -------------------------------------------------------------
        [ERROR] COMPILATION ERROR :
        [INFO] -------------------------------------------------------------
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[136,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[138,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[139,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[140,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[141,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[142,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[143,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[144,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[145,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[146,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[147,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[148,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[149,0] class, interface, or enum expected
        [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java:[150,0] class, interface, or enum expected
        [INFO] 14 errors
        [INFO] -------------------------------------------------------------
        [INFO] ------------------------------------------------------------------------
        [INFO] BUILD FAILURE
        [INFO] -------------------------------------------------------------------

        Show
        Avery Ching added a comment - Claudio, Just checked out your diff. Overall looks pretty good, cleans up the code nicely. However, I am having the same trouble building it that I mentioned in GIRAPH-64 . You don't have this issue? mvn package [INFO] Scanning for projects... [INFO] [INFO] ------------------------------------------------------------------------ [INFO] Building Apache Incubator Giraph 0.70 [INFO] ------------------------------------------------------------------------ [INFO] [INFO] — maven-enforcer-plugin:1.0.1:enforce (enforce-maven) @ giraph — [INFO] [INFO] — maven-resources-plugin:2.4.3:resources (default-resources) @ giraph — [INFO] Using 'UTF-8' encoding to copy filtered resources. [INFO] skip non existing resourceDirectory /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/resources [INFO] [INFO] — maven-compiler-plugin:2.3.2:compile (default-compile) @ giraph — [INFO] Compiling 90 source files to /Users/aching/Documents/workspace/Trunk Apache Giraph/target/classes [INFO] ------------------------------------------------------------- [ERROR] COMPILATION ERROR : [INFO] ------------------------------------------------------------- [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [136,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [138,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [139,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [140,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [141,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [142,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [143,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [144,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [145,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [146,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [147,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [148,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [149,0] class, interface, or enum expected [ERROR] /Users/aching/Documents/workspace/Trunk Apache Giraph/src/main/java/org/apache/giraph/GiraphRunner.java: [150,0] class, interface, or enum expected [INFO] 14 errors [INFO] ------------------------------------------------------------- [INFO] ------------------------------------------------------------------------ [INFO] BUILD FAILURE [INFO] -------------------------------------------------------------------
        Hide
        Claudio Martella added a comment -

        No, it compiles, installs and packages cleanly. I don't have GiraphRunner.java though. I'm synched with friday's trunk, I think.

        Show
        Claudio Martella added a comment - No, it compiles, installs and packages cleanly. I don't have GiraphRunner.java though. I'm synched with friday's trunk, I think.
        Hide
        Claudio Martella added a comment -

        Once this is committed, i'd open a new issue to remove AggregatorUsage implementation from BasicVertex, it doesn't make much sense anymore (same as with getContext, or?).

        Show
        Claudio Martella added a comment - Once this is committed, i'd open a new issue to remove AggregatorUsage implementation from BasicVertex, it doesn't make much sense anymore (same as with getContext, or?).
        Hide
        Jakob Homan added a comment -

        This looks good but I have some comments when not typing on glass. Avery - did you do a git clean before applying? Sounds like you still have the new files from 64 in the tree.

        Show
        Jakob Homan added a comment - This looks good but I have some comments when not typing on glass. Avery - did you do a git clean before applying? Sounds like you still have the new files from 64 in the tree.
        Hide
        Avery Ching added a comment -

        Sorry, Claudio, my bad. I still had remnants of GIRAPH-64. It's fine and passes local unittests.

        Show
        Avery Ching added a comment - Sorry, Claudio, my bad. I still had remnants of GIRAPH-64 . It's fine and passes local unittests.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2746/
        -----------------------------------------------------------

        Review request for giraph.

        Summary
        -------

        Claudio's patch for GIRAPH-47.

        This addresses bug GIRAPH-47.
        https://issues.apache.org/jira/browse/GIRAPH-47

        Diffs


        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java PRE-CREATION
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java PRE-CREATION
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java PRE-CREATION
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java 1198865
        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java 1198865

        Diff: https://reviews.apache.org/r/2746/diff

        Testing
        -------

        mvn install

        Thanks,

        Avery

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2746/ ----------------------------------------------------------- Review request for giraph. Summary ------- Claudio's patch for GIRAPH-47 . This addresses bug GIRAPH-47 . https://issues.apache.org/jira/browse/GIRAPH-47 Diffs http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java 1198865 Diff: https://reviews.apache.org/r/2746/diff Testing ------- mvn install Thanks, Avery
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2746/#review3082
        -----------------------------------------------------------

        Claudio, really nice stuff here. Most of my comments are related to indenting. But otherwise, this is a lot better IMO. Please take a look at CODE_CONVENTIONS and fix accordingly. While the official policy is 2 space, at this time, for the 4 space indented files, please keep to 4 spaces for consistency. We will transition everything over at some point. New files can be 2 space (new convention) if desired.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
        <https://reviews.apache.org/r/2746/#comment6885>

        This doesn't need to be static anymore.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
        <https://reviews.apache.org/r/2746/#comment6870>

        Indenting should be 8 spaces.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
        <https://reviews.apache.org/r/2746/#comment6873>

        extra line.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
        <https://reviews.apache.org/r/2746/#comment6874>

        extra line.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
        <https://reviews.apache.org/r/2746/#comment6875>

        4 spaces.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
        <https://reviews.apache.org/r/2746/#comment6871>

        4 spaces indenting.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
        <https://reviews.apache.org/r/2746/#comment6872>

        4 spaces indenting.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
        <https://reviews.apache.org/r/2746/#comment6876>

        Align to GiraphJob.WORKER_CONTEXT_CLASS

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java
        <https://reviews.apache.org/r/2746/#comment6877>

        VERTEX_COUNT shouldn't be capitalized. All caps should be reserved for only static values.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java
        <https://reviews.apache.org/r/2746/#comment6878>

        EDGE_COUNT shouldn't be capitalized. All caps should be reserved for only static values.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java
        <https://reviews.apache.org/r/2746/#comment6887>

        These no longer need to be static anymore, could be private variables that have public accessor method.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java
        <https://reviews.apache.org/r/2746/#comment6879>

        Might want to add a comment about this example. I.e.

        /**

        • Fully runnable example of how to
        • emit worker data to HDFS during a graph
        • computation.
          */

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java
        <https://reviews.apache.org/r/2746/#comment6880>

        extra line.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java
        <https://reviews.apache.org/r/2746/#comment6881>

        Awesome, I hated this.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
        <https://reviews.apache.org/r/2746/#comment6882>

        indenting.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java
        <https://reviews.apache.org/r/2746/#comment6883>

        extra line.

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java
        <https://reviews.apache.org/r/2746/#comment6884>

        Other javadoc has lines in between comment and params (i.e.

        • superstep starts.
          *
        • @throws IllegalAccessException
        • Avery

        On 2011-11-07 19:09:08, Avery Ching wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/2746/

        -----------------------------------------------------------

        (Updated 2011-11-07 19:09:08)

        Review request for giraph.

        Summary

        -------

        Claudio's patch for GIRAPH-47.

        This addresses bug GIRAPH-47.

        https://issues.apache.org/jira/browse/GIRAPH-47

        Diffs

        -----

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java PRE-CREATION

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java PRE-CREATION

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java PRE-CREATION

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java 1198865

        Diff: https://reviews.apache.org/r/2746/diff

        Testing

        -------

        mvn install

        Thanks,

        Avery

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2746/#review3082 ----------------------------------------------------------- Claudio, really nice stuff here. Most of my comments are related to indenting. But otherwise, this is a lot better IMO. Please take a look at CODE_CONVENTIONS and fix accordingly. While the official policy is 2 space, at this time, for the 4 space indented files, please keep to 4 spaces for consistency. We will transition everything over at some point. New files can be 2 space (new convention) if desired. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java < https://reviews.apache.org/r/2746/#comment6885 > This doesn't need to be static anymore. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java < https://reviews.apache.org/r/2746/#comment6870 > Indenting should be 8 spaces. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java < https://reviews.apache.org/r/2746/#comment6873 > extra line. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java < https://reviews.apache.org/r/2746/#comment6874 > extra line. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java < https://reviews.apache.org/r/2746/#comment6875 > 4 spaces. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java < https://reviews.apache.org/r/2746/#comment6871 > 4 spaces indenting. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java < https://reviews.apache.org/r/2746/#comment6872 > 4 spaces indenting. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java < https://reviews.apache.org/r/2746/#comment6876 > Align to GiraphJob.WORKER_CONTEXT_CLASS http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java < https://reviews.apache.org/r/2746/#comment6877 > VERTEX_COUNT shouldn't be capitalized. All caps should be reserved for only static values. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java < https://reviews.apache.org/r/2746/#comment6878 > EDGE_COUNT shouldn't be capitalized. All caps should be reserved for only static values. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java < https://reviews.apache.org/r/2746/#comment6887 > These no longer need to be static anymore, could be private variables that have public accessor method. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java < https://reviews.apache.org/r/2746/#comment6879 > Might want to add a comment about this example. I.e. /** Fully runnable example of how to emit worker data to HDFS during a graph computation. */ http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java < https://reviews.apache.org/r/2746/#comment6880 > extra line. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java < https://reviews.apache.org/r/2746/#comment6881 > Awesome, I hated this. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java < https://reviews.apache.org/r/2746/#comment6882 > indenting. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java < https://reviews.apache.org/r/2746/#comment6883 > extra line. http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java < https://reviews.apache.org/r/2746/#comment6884 > Other javadoc has lines in between comment and params (i.e. superstep starts. * @throws IllegalAccessException Avery On 2011-11-07 19:09:08, Avery Ching wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2746/ ----------------------------------------------------------- (Updated 2011-11-07 19:09:08) Review request for giraph. Summary ------- Claudio's patch for GIRAPH-47 . This addresses bug GIRAPH-47 . https://issues.apache.org/jira/browse/GIRAPH-47 Diffs ----- http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java 1198865 Diff: https://reviews.apache.org/r/2746/diff Testing ------- mvn install Thanks, Avery
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-11-07 19:12:55, Avery Ching wrote:

        > Claudio, really nice stuff here. Most of my comments are related to indenting. But otherwise, this is a lot better IMO. Please take a look at CODE_CONVENTIONS and fix accordingly. While the official policy is 2 space, at this time, for the 4 space indented files, please keep to 4 spaces for consistency. We will transition everything over at some point. New files can be 2 space (new convention) if desired.

        Ok, still have to understand a bit the code conventions. Trying to stick to them. Maybe an Eclipse format conf file would help? Could you share yours, if you have one?

        On 2011-11-07 19:12:55, Avery Ching wrote:

        > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java, lines 91-92

        > <https://reviews.apache.org/r/2746/diff/1/?file=56634#file56634line91>

        >

        > These no longer need to be static anymore, could be private variables that have public accessor method.

        Not sure we can do this. How will tests get to their values. Can't access those members if not static.

        On 2011-11-07 19:12:55, Avery Ching wrote:

        > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java, lines 250-251

        > <https://reviews.apache.org/r/2746/diff/1/?file=56632#file56632line250>

        >

        > Align to GiraphJob.WORKER_CONTEXT_CLASS

        What do you mean? I aligned to the example, all classes are set with .setClass() there. Fixing the whole thing.

        On 2011-11-07 19:12:55, Avery Ching wrote:

        > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java, line 150

        > <https://reviews.apache.org/r/2746/diff/1/?file=56632#file56632line150>

        >

        > This doesn't need to be static anymore.

        Can't make it non static. Won't be able to read from tests.

        • Claudio

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2746/#review3082
        -----------------------------------------------------------

        On 2011-11-07 19:09:08, Avery Ching wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/2746/

        -----------------------------------------------------------

        (Updated 2011-11-07 19:09:08)

        Review request for giraph.

        Summary

        -------

        Claudio's patch for GIRAPH-47.

        This addresses bug GIRAPH-47.

        https://issues.apache.org/jira/browse/GIRAPH-47

        Diffs

        -----

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java PRE-CREATION

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java PRE-CREATION

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java PRE-CREATION

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java 1198865

        Diff: https://reviews.apache.org/r/2746/diff

        Testing

        -------

        mvn install

        Thanks,

        Avery

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-11-07 19:12:55, Avery Ching wrote: > Claudio, really nice stuff here. Most of my comments are related to indenting. But otherwise, this is a lot better IMO. Please take a look at CODE_CONVENTIONS and fix accordingly. While the official policy is 2 space, at this time, for the 4 space indented files, please keep to 4 spaces for consistency. We will transition everything over at some point. New files can be 2 space (new convention) if desired. Ok, still have to understand a bit the code conventions. Trying to stick to them. Maybe an Eclipse format conf file would help? Could you share yours, if you have one? On 2011-11-07 19:12:55, Avery Ching wrote: > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java , lines 91-92 > < https://reviews.apache.org/r/2746/diff/1/?file=56634#file56634line91 > > > These no longer need to be static anymore, could be private variables that have public accessor method. Not sure we can do this. How will tests get to their values. Can't access those members if not static. On 2011-11-07 19:12:55, Avery Ching wrote: > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java , lines 250-251 > < https://reviews.apache.org/r/2746/diff/1/?file=56632#file56632line250 > > > Align to GiraphJob.WORKER_CONTEXT_CLASS What do you mean? I aligned to the example, all classes are set with .setClass() there. Fixing the whole thing. On 2011-11-07 19:12:55, Avery Ching wrote: > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java , line 150 > < https://reviews.apache.org/r/2746/diff/1/?file=56632#file56632line150 > > > This doesn't need to be static anymore. Can't make it non static. Won't be able to read from tests. Claudio ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2746/#review3082 ----------------------------------------------------------- On 2011-11-07 19:09:08, Avery Ching wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2746/ ----------------------------------------------------------- (Updated 2011-11-07 19:09:08) Review request for giraph. Summary ------- Claudio's patch for GIRAPH-47 . This addresses bug GIRAPH-47 . https://issues.apache.org/jira/browse/GIRAPH-47 Diffs ----- http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java 1198865 Diff: https://reviews.apache.org/r/2746/diff Testing ------- mvn install Thanks, Avery
        Hide
        Claudio Martella added a comment -

        Should fix the issues from review. Also, I added the implementation of AggregatorUsage to WorkerContext.

        Show
        Claudio Martella added a comment - Should fix the issues from review. Also, I added the implementation of AggregatorUsage to WorkerContext.
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-11-07 19:12:55, Avery Ching wrote:

        > Claudio, really nice stuff here. Most of my comments are related to indenting. But otherwise, this is a lot better IMO. Please take a look at CODE_CONVENTIONS and fix accordingly. While the official policy is 2 space, at this time, for the 4 space indented files, please keep to 4 spaces for consistency. We will transition everything over at some point. New files can be 2 space (new convention) if desired.

        Claudio Martella wrote:

        Ok, still have to understand a bit the code conventions. Trying to stick to them. Maybe an Eclipse format conf file would help? Could you share yours, if you have one?

        Mine is all messed up too. I have to manually fix some things.

        On 2011-11-07 19:12:55, Avery Ching wrote:

        > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java, line 150

        > <https://reviews.apache.org/r/2746/diff/1/?file=56632#file56632line150>

        >

        > This doesn't need to be static anymore.

        Claudio Martella wrote:

        Can't make it non static. Won't be able to read from tests.

        Sorry I wasn't more clear, I was suggesting that we fix this. But it's not really related to this issue. So don't worry about it. Please ignore my comment.

        On 2011-11-07 19:12:55, Avery Ching wrote:

        > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java, lines 91-92

        > <https://reviews.apache.org/r/2746/diff/1/?file=56634#file56634line91>

        >

        > These no longer need to be static anymore, could be private variables that have public accessor method.

        Claudio Martella wrote:

        Not sure we can do this. How will tests get to their values. Can't access those members if not static.

        I was suggesting that we fix this, maybe give the user the worker context at the end. Actually not sure it's the right solution and it's not really related to this issue. So don't worry about it. Please ignore my comment.

        • Avery

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2746/#review3082
        -----------------------------------------------------------

        On 2011-11-07 19:09:08, Avery Ching wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/2746/

        -----------------------------------------------------------

        (Updated 2011-11-07 19:09:08)

        Review request for giraph.

        Summary

        -------

        Claudio's patch for GIRAPH-47.

        This addresses bug GIRAPH-47.

        https://issues.apache.org/jira/browse/GIRAPH-47

        Diffs

        -----

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java PRE-CREATION

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java PRE-CREATION

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java PRE-CREATION

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java 1198865

        http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java 1198865

        Diff: https://reviews.apache.org/r/2746/diff

        Testing

        -------

        mvn install

        Thanks,

        Avery

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-11-07 19:12:55, Avery Ching wrote: > Claudio, really nice stuff here. Most of my comments are related to indenting. But otherwise, this is a lot better IMO. Please take a look at CODE_CONVENTIONS and fix accordingly. While the official policy is 2 space, at this time, for the 4 space indented files, please keep to 4 spaces for consistency. We will transition everything over at some point. New files can be 2 space (new convention) if desired. Claudio Martella wrote: Ok, still have to understand a bit the code conventions. Trying to stick to them. Maybe an Eclipse format conf file would help? Could you share yours, if you have one? Mine is all messed up too. I have to manually fix some things. On 2011-11-07 19:12:55, Avery Ching wrote: > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java , line 150 > < https://reviews.apache.org/r/2746/diff/1/?file=56632#file56632line150 > > > This doesn't need to be static anymore. Claudio Martella wrote: Can't make it non static. Won't be able to read from tests. Sorry I wasn't more clear, I was suggesting that we fix this. But it's not really related to this issue. So don't worry about it. Please ignore my comment. On 2011-11-07 19:12:55, Avery Ching wrote: > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java , lines 91-92 > < https://reviews.apache.org/r/2746/diff/1/?file=56634#file56634line91 > > > These no longer need to be static anymore, could be private variables that have public accessor method. Claudio Martella wrote: Not sure we can do this. How will tests get to their values. Can't access those members if not static. I was suggesting that we fix this, maybe give the user the worker context at the end. Actually not sure it's the right solution and it's not really related to this issue. So don't worry about it. Please ignore my comment. Avery ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2746/#review3082 ----------------------------------------------------------- On 2011-11-07 19:09:08, Avery Ching wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2746/ ----------------------------------------------------------- (Updated 2011-11-07 19:09:08) Review request for giraph. Summary ------- Claudio's patch for GIRAPH-47 . This addresses bug GIRAPH-47 . https://issues.apache.org/jira/browse/GIRAPH-47 Diffs ----- http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java 1198865 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java 1198865 Diff: https://reviews.apache.org/r/2746/diff Testing ------- mvn install Thanks, Avery
        Hide
        Avery Ching added a comment -

        +1
        Thanks Claudio. I'm going to commit on your behalf. I made a few small changes to fix the code conventions. I also fixed a bug in the distributed unittests (WorkerContext not set for TestAutoCheckpoint). The local unittests can't catch that one. Next time, you'll want to run against a real Hadoop instance to catch problems (i.e. mvn test -Dprop.mapred.job.tracker=localhost:50300). You'll want to replace 'localhost:50300' with your actual Hadoop jobtracker and port settings.

        Show
        Avery Ching added a comment - +1 Thanks Claudio. I'm going to commit on your behalf. I made a few small changes to fix the code conventions. I also fixed a bug in the distributed unittests (WorkerContext not set for TestAutoCheckpoint). The local unittests can't catch that one. Next time, you'll want to run against a real Hadoop instance to catch problems (i.e. mvn test -Dprop.mapred.job.tracker=localhost:50300). You'll want to replace 'localhost:50300' with your actual Hadoop jobtracker and port settings.
        Hide
        Hudson added a comment -

        Integrated in Giraph-trunk-Commit #24 (See https://builds.apache.org/job/Giraph-trunk-Commit/24/)
        GIRAPH-47: Export Worker's Context/State to vertices through
        pre/post/Application/Superstep. (cmartella via aching)

        aching : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1198972
        Files :

        • /incubator/giraph/trunk/CHANGELOG
        • /incubator/giraph/trunk/CODE_CONVENTIONS
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java
        • /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestAutoCheckpoint.java
        • /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java
        • /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java
        • /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java
        • /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java
        Show
        Hudson added a comment - Integrated in Giraph-trunk-Commit #24 (See https://builds.apache.org/job/Giraph-trunk-Commit/24/ ) GIRAPH-47 : Export Worker's Context/State to vertices through pre/post/Application/Superstep. (cmartella via aching) aching : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1198972 Files : /incubator/giraph/trunk/CHANGELOG /incubator/giraph/trunk/CODE_CONVENTIONS /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestAutoCheckpoint.java /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java
        Hide
        Avery Ching added a comment -

        I filed issue https://issues.apache.org/jira/browse/INFRA-4097.

        Something is weird with hudson, cannot find mvn.

        https://builds.apache.org/job/Giraph-trunk-Commit/24/console
        <snip>
        [Giraph-trunk-Commit] $ /bin/bash -x /tmp/hudson6174557603739055037.sh
        + export JAVA_HOME=/home/jenkins/tools/java/latest
        + JAVA_HOME=/home/jenkins/tools/java/latest
        + export MAVEN_HOME=/home/jenkins/tools/maven/latest
        + MAVEN_HOME=/home/jenkins/tools/maven/latest
        + export PATH=/home/jenkins/tools/java/latest/bin:/home/jenkins/tools/maven/latest/bin:/home/hudson/tools/java/latest1.6/bin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/games:/home/hudson/.rvm/bin
        + PATH=/home/jenkins/tools/java/latest/bin:/home/jenkins/tools/maven/latest/bin:/home/hudson/tools/java/latest1.6/bin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/games:/home/hudson/.rvm/bin
        + which java
        /home/hudson/tools/java/latest1.6/bin/java
        + cd /home/hudson/hudson-slave/workspace/Giraph-trunk-Commit/trunk
        + /home/jenkins/tools/maven/latest/bin/mvn clean compile test
        /tmp/hudson6174557603739055037.sh: line 9: /home/jenkins/tools/maven/latest/bin/mvn: No such file or directory
        Build step 'Execute shell' marked build as failure
        Updating GIRAPH-47
        Recording test results
        Finished: FAILURE

        Show
        Avery Ching added a comment - I filed issue https://issues.apache.org/jira/browse/INFRA-4097 . Something is weird with hudson, cannot find mvn. https://builds.apache.org/job/Giraph-trunk-Commit/24/console <snip> [Giraph-trunk-Commit] $ /bin/bash -x /tmp/hudson6174557603739055037.sh + export JAVA_HOME=/home/jenkins/tools/java/latest + JAVA_HOME=/home/jenkins/tools/java/latest + export MAVEN_HOME=/home/jenkins/tools/maven/latest + MAVEN_HOME=/home/jenkins/tools/maven/latest + export PATH=/home/jenkins/tools/java/latest/bin:/home/jenkins/tools/maven/latest/bin:/home/hudson/tools/java/latest1.6/bin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/games:/home/hudson/.rvm/bin + PATH=/home/jenkins/tools/java/latest/bin:/home/jenkins/tools/maven/latest/bin:/home/hudson/tools/java/latest1.6/bin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/games:/home/hudson/.rvm/bin + which java /home/hudson/tools/java/latest1.6/bin/java + cd /home/hudson/hudson-slave/workspace/Giraph-trunk-Commit/trunk + /home/jenkins/tools/maven/latest/bin/mvn clean compile test /tmp/hudson6174557603739055037.sh: line 9: /home/jenkins/tools/maven/latest/bin/mvn: No such file or directory Build step 'Execute shell' marked build as failure Updating GIRAPH-47 Recording test results Finished: FAILURE
        Hide
        Jakob Homan added a comment -

        Avery, can attach the final diff that you committed to the tree, for bookkeeping purposes?

        Show
        Jakob Homan added a comment - Avery, can attach the final diff that you committed to the tree, for bookkeeping purposes?
        Hide
        Jakob Homan added a comment -

        Avery, can attach the final diff that you committed to the tree, for bookkeeping purposes?

        Show
        Jakob Homan added a comment - Avery, can attach the final diff that you committed to the tree, for bookkeeping purposes?
        Hide
        Avery Ching added a comment -

        Here's the final diff. Sorry about forgetting to attach it.

        Show
        Avery Ching added a comment - Here's the final diff. Sorry about forgetting to attach it.

          People

          • Assignee:
            Claudio Martella
            Reporter:
            Claudio Martella
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development