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

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.
bq.  
bq.  Claudio Martella wrote:
bq.      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.


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.
bq.  
bq.  Claudio Martella wrote:
bq.      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.


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.
bq.  
bq.  Claudio Martella wrote:
bq.      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:
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, 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