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


/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java
line 189, 192: broken code conventions, put a new line after you open the block 
and close at new line as well. the convention you're using is for fields. Also 
please explain with one sentence about the semantic of the parameter and how it 
is supposed to use it. users might not read the JIRAs.

For the rest I'm happy with it, nice work.
I don't have availability of real cluster here at the moment, if somebody can 
launch the testing on a real cluster I'd be ready to commit this one. 
Otherwise I should be able to run in on monday or so.

- Claudio


On 2011-12-15 10:42:39, Sebastian Schelter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3203/
> -----------------------------------------------------------
> 
> (Updated 2011-12-15 10:42:39)
> 
> 
> Review request for giraph.
> 
> 
> Summary
> -------
> 
> refactoring that gives BasicVertex this 3 new methods:
> 
>     public abstract Iterable<M> getMessages()
> 
> returns an unmodifiable iterable allowing access to the current messages
> 
>     public abstract void setMessages(Iterable<M> messages);
> 
> replacement for getMsgList().clear() followed by getMsgList().addAll(...);
> 
>     public abstract void releaseResources();
> 
> after a vertex voted to halt, all references to messages it could still hold 
> should be removed to enable earlier GC, instead of externally calling 
> replacement for getMsgList().clear(), this method should be used
> 
> Local unit tests pass, unfortunately I wasn't yet able to run the tests on my 
> hadoop cluster (still have problems because I can only access it via a socks 
> proxy)
> 
> 
> This addresses bug GIRAPH-80.
>     https://issues.apache.org/jira/browse/GIRAPH-80
> 
> 
> Diffs
> -----
> 
>   /trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 
> 1214675 
>   /trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1214675 
>   /trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java 
> 1214675 
>   /trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 1214675 
>   /trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1214675 
>   
> /trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 
> 1214675 
>   /trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1214675 
>   /trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java 1214675 
>   /trunk/src/main/java/org/apache/giraph/utils/ComparisonUtils.java 
> PRE-CREATION 
>   /trunk/src/main/java/org/apache/giraph/utils/MemoryUtils.java 1214675 
>   /trunk/src/test/java/org/apache/giraph/utils/ComparisonUtilsTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3203/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sebastian
> 
>

Reply via email to