Review Board 1.7.22


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

Review Request #2746 - Created Nov. 7, 2011 and discarded

Avery Ching
trunk
GIRAPH-47
Reviewers
giraph
giraph
Claudio's patch for GIRAPH-47.
mvn install
Posted (Nov. 7, 2011, 7:12 p.m.)
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.
  1. 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?
  2. Mine is all messed up too.  I have to manually fix some things.
This doesn't need to be static anymore.
  1. Can't make it non static. Won't be able to read from tests.
  2. 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.
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java (Diff revision 1)
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
Indenting should be 8 spaces.
extra line.
extra line.
4 spaces.
4 spaces indenting.
4 spaces indenting.
Align to GiraphJob.WORKER_CONTEXT_CLASS
  1. What do you mean? I aligned to the example, all classes are set with .setClass() there. Fixing the whole thing.
VERTEX_COUNT shouldn't be capitalized.  All caps should be reserved for only static values.
EDGE_COUNT shouldn't be capitalized.  All caps should be reserved for only static values.
These no longer need to be static anymore, could be private variables that have public accessor method.
  1. Not sure we can do this. How will tests get to their values. Can't access those members if not static.
  2. 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.
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.
 */
extra line.
Awesome, I hated this.
indenting.
extra line.
Other javadoc has lines in between comment and params (i.e.

* superstep starts.
*
* @throws IllegalAccessException