giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Avery Ching" <avery.ch...@gmail.com>
Subject Re: Review Request: Allow creation of graph by adding edges that span multiple workers
Date Fri, 02 Nov 2012 21:12:10 GMT

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


This is a lot of great work.  Really looking forward to seeing this functionality get rid
of preparation time.  Thanks for the previous review Nitay as well.  My comments are all pretty
minor.


/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java
<https://reviews.apache.org/r/7784/#comment28102>

    extra line.



/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java
<https://reviews.apache.org/r/7784/#comment28103>

    prefer this in the exception to avoid runtime overhead without exceptions.
    
    



/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java
<https://reviews.apache.org/r/7784/#comment28105>

    This is great code sharing.



/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java
<https://reviews.apache.org/r/7784/#comment28104>

    same here.



/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
<https://reviews.apache.org/r/7784/#comment28106>

    This isn't necessary due to the previous fix.



/trunk/giraph/src/main/java/org/apache/giraph/graph/BspUtils.java
<https://reviews.apache.org/r/7784/#comment28107>

    why do you need these?  You already implemented this in ImmutableClassesGiraphConfiguration.



/trunk/giraph/src/main/java/org/apache/giraph/graph/EdgeWithSource.java
<https://reviews.apache.org/r/7784/#comment28108>

    these can be final here.



/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitEvents.java
<https://reviews.apache.org/r/7784/#comment28109>

    This argument can be Progressable instead (cleaner).



/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java
<https://reviews.apache.org/r/7784/#comment28118>

    extra space? ) {



/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java
<https://reviews.apache.org/r/7784/#comment28111>

    InputSplitsCallable.



/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java
<https://reviews.apache.org/r/7784/#comment28112>

    InputSplitsCallable.



/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java
<https://reviews.apache.org/r/7784/#comment28113>

    ?



/trunk/giraph/src/main/java/org/apache/giraph/graph/VertexInputSplitsCallable.java
<https://reviews.apache.org/r/7784/#comment28114>

    indent



/trunk/giraph/src/main/java/org/apache/giraph/io/TextVertexInputFormat.java
<https://reviews.apache.org/r/7784/#comment28115>

    Awesome.



/trunk/giraph/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java
<https://reviews.apache.org/r/7784/#comment28116>

    Haha, that is a lot of parameters.



/trunk/giraph/src/test/java/org/apache/giraph/TestEdgeInput.java
<https://reviews.apache.org/r/7784/#comment28117>

    You always write the best unit tests Alessandro =)


- Avery Ching


On Nov. 2, 2012, 3:56 p.m., Alessandro Presta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7784/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2012, 3:56 p.m.)
> 
> 
> Review request for giraph.
> 
> 
> Description
> -------
> 
> Allow creation of graph by adding edges that span multiple workers
> 
> 
> This addresses bug GIRAPH-155.
>     https://issues.apache.org/jira/browse/GIRAPH-155
> 
> 
> Diffs
> -----
> 
>   /trunk/giraph/src/main/java/org/apache/giraph/GiraphConfiguration.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java
1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 1404751

>   /trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerServer.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java
1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java 1404751

>   /trunk/giraph/src/main/java/org/apache/giraph/comm/requests/SendPartitionMutationsRequest.java
1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/BasicVertexValueReader.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/BspUtils.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/EdgeInputFormat.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/graph/EdgeInputSplitsCallable.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/graph/EdgeInputSplitsCallableFactory.java
PRE-CREATION 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/EdgeReader.java PRE-CREATION 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/EdgeWithSource.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/graph/GiraphInputFormat.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/graph/GiraphJob.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/GraphMapper.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitEvents.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitPaths.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java 1404751

>   /trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallableFactory.java
PRE-CREATION 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexInputFormat.java 1404751

>   /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexInputSplitsCallable.java
PRE-CREATION 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexInputSplitsCallableFactory.java
PRE-CREATION 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexReader.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexResolver.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexValueInputFormat.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexValueReader.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/io/GiraphFileInputFormat.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/io/GiraphTextInputFormat.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/io/IntIntTextVertexValueInputFormat.java
PRE-CREATION 
>   /trunk/giraph/src/main/java/org/apache/giraph/io/IntNullTextEdgeInputFormat.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/io/TextEdgeInputFormat.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/io/TextVertexInputFormat.java 1404751

>   /trunk/giraph/src/main/java/org/apache/giraph/io/TextVertexValueInputFormat.java PRE-CREATION

>   /trunk/giraph/src/main/java/org/apache/giraph/utils/FileUtils.java 1404751 
>   /trunk/giraph/src/main/java/org/apache/giraph/utils/IntPair.java PRE-CREATION 
>   /trunk/giraph/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java 1404751

>   /trunk/giraph/src/test/java/org/apache/giraph/BspCase.java 1404751 
>   /trunk/giraph/src/test/java/org/apache/giraph/TestEdgeInput.java PRE-CREATION 
>   /trunk/giraph/src/test/java/org/apache/giraph/TestJsonBase64Format.java 1404751 
> 
> Diff: https://reviews.apache.org/r/7784/diff/
> 
> 
> Testing
> -------
> 
> 1) mvn verify
> 2) pseudo-distributed tests
> 3) PageRank on cluster with real text input (will post perf results on the issue)
> 
> 
> Thanks,
> 
> Alessandro Presta
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message