giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alessandro Presta" <alessan...@fb.com>
Subject Re: Review Request: Vertex API redesign
Date Mon, 16 Jul 2012 13:34:12 GMT


> On July 16, 2012, 1:15 p.m., Claudio Martella wrote:
> >

Thanks for re-opening the issues from the old review. This way we have everything in one place.


> On July 16, 2012, 1:15 p.m., Claudio Martella wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java,
line 177
> > <https://reviews.apache.org/r/5984/diff/1/?file=123119#file123119line177>
> >
> >     shouldn't this have Iterable<Text>?

Thanks, I guess I've missed it because *-contrib is not compiled by maven.


> On July 16, 2012, 1:15 p.m., Claudio Martella wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java,
line 163
> > <https://reviews.apache.org/r/5984/diff/1/?file=123131#file123131line163>
> >
> >     Use Guava?

Will fix all of those. Was just wondering what our convention is here :)


> On July 16, 2012, 1:15 p.m., Claudio Martella wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ServerData.java,
line 46
> > <https://reviews.apache.org/r/5984/diff/1/?file=123138#file123138line46>
> >
> >     Does Guava provide a method for its creation?

Not really for a ConcurrentHashMap, as far as I know.


> On July 16, 2012, 1:15 p.m., Claudio Martella wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java,
line 106
> > <https://reviews.apache.org/r/5984/diff/1/?file=123158#file123158line106>
> >
> >     I agree with Avery here, it's not obvious that allowing userland to change the
vertex id under the hood is error-proof

But then why should it be available in MutableVertex? To my understanding, MutableVertex is
about mutating the graph, not the identity of vertices.
Perhaps we should have a default initialize() that user-defined initialize() can call? Not
that elegant, and nothing prevents a user from calling initialize() more than once, although
the naming should advise against it.

Another option could be to enforce that setId() only changes the id if it's currently null.
We could also output a warning. What do you think?


- Alessandro


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


On July 16, 2012, 12:46 p.m., Alessandro Presta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5984/
> -----------------------------------------------------------
> 
> (Updated July 16, 2012, 12:46 p.m.)
> 
> 
> Review request for giraph, Avery Ching and Claudio Martella.
> 
> 
> Description
> -------
> 
> Please see https://issues.apache.org/jira/browse/GIRAPH-244
> 
> 
> This addresses bug GIRAPH-244.
>     https://issues.apache.org/jira/browse/GIRAPH-244
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/HashMapVertexPageRankBenchmark.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankComputation.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/CommunicationsInterface.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendMutationsCache.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ServerData.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/VertexList.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerClient.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerServer.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/ConnectedComponentsVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/IntIntNullIntTextInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCombinerVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleFailVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMasterComputeVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMsgVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSuperstepVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleTextVertexOutputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleTriangleClosingVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/VerifyMessage.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/VertexWithComponentTextOutputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/Edge.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeWritable.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/HashMapVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/MutableVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/SimpleMutableVertex.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/SimpleVertex.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexChanges.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexMutations.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexReader.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexWriter.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/lib/AdjacencyListTextVertexOutputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/lib/AdjacencyListVertexReader.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/lib/IdWithValueTextOutputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/lib/JsonBase64VertexInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/lib/JsonBase64VertexOutputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/lib/JsonLongDoubleFloatDoubleVertexInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/lib/JsonLongDoubleFloatDoubleVertexOutputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/lib/LongDoubleDoubleAdjacencyListVertexInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/lib/SequenceFileVertexInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/lib/TextDoubleDoubleAdjacencyListVertexInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestVertexTypes.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/examples/SimpleShortestPathsVertexTest.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/examples/SimpleTriangleClosingVertexTest.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/graph/TestEdgeListVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/graph/TestIntIntNullIntVertex.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/lib/TestAdjacencyListTextVertexOutputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/lib/TestIdWithValueTextOutputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/lib/TestLongDoubleDoubleAdjacencyListVertexInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/lib/TestTextDoubleDoubleAdjacencyListVertexInputFormat.java
1362000 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/utils/MockUtils.java
1362000 
> 
> Diff: https://reviews.apache.org/r/5984/diff/
> 
> 
> Testing
> -------
> 
> mvn verify
> 
> 
> Thanks,
> 
> Alessandro Presta
> 
>


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