[ 
https://issues.apache.org/jira/browse/GIRAPH-47?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13145754#comment-13145754
 ] 

jirapos...@reviews.apache.org commented on GIRAPH-47:
-----------------------------------------------------



bq.  On 2011-11-07 19:12:55, Avery Ching wrote:
bq.  > 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?


bq.  On 2011-11-07 19:12:55, Avery Ching wrote:
bq.  > 
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java,
 lines 91-92
bq.  > <https://reviews.apache.org/r/2746/diff/1/?file=56634#file56634line91>
bq.  >
bq.  >     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.


bq.  On 2011-11-07 19:12:55, Avery Ching wrote:
bq.  > 
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java,
 lines 250-251
bq.  > <https://reviews.apache.org/r/2746/diff/1/?file=56632#file56632line250>
bq.  >
bq.  >     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.


bq.  On 2011-11-07 19:12:55, Avery Ching wrote:
bq.  > 
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java,
 line 150
bq.  > <https://reviews.apache.org/r/2746/diff/1/?file=56632#file56632line150>
bq.  >
bq.  >     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:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2746/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-11-07 19:09:08)
bq.  
bq.  
bq.  Review request for giraph.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Claudio's patch for GIRAPH-47.
bq.  
bq.  
bq.  This addresses bug GIRAPH-47.
bq.      https://issues.apache.org/jira/browse/GIRAPH-47
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedService.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/DefaultWorkerContext.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerContext.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java
 1198865 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexRangeBalancer.java
 1198865 
bq.  
bq.  Diff: https://reviews.apache.org/r/2746/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  mvn install
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Avery
bq.  
bq.


                
> Export Worker's Context/State to vertices through 
> pre/post/Application/Superstep
> --------------------------------------------------------------------------------
>
>                 Key: GIRAPH-47
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-47
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Claudio Martella
>            Assignee: Claudio Martella
>         Attachments: GIRAPH-47.diff, GIRAPH-47.diff
>
>
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to