if you need a cluster run let me know I'd be happy to take another whack at it too. Our clusters our clearing up nicely...!
On Tue, Aug 21, 2012 at 12:21 AM, Avery Ching <[email protected]> wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5987/#review10570 > ----------------------------------------------------------- > > > This is a really great way to add a terrific features while improving the > codebase. Looks very nice and got rid of a lot of bad stuff. > > There is a compile issue though: > > [ERROR] COMPILATION ERROR : > [INFO] ------------------------------------------------------------- > [ERROR] > /Users/aching/git/git_svn_giraph_trunk/target/munged/test/org/apache/giraph/TestMutateGraphVertex.java:[34,7] > class TestMutateGraph is public, should be declared in a file named > TestMutateGraph.java > > Does this happen for you? > > Would you mind trying out the distributed tests and run a few benchmarks > on a real cluster just to verify? > > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java > <https://reviews.apache.org/r/5987/#comment22617> > > This is way cleaner. Definitely pleased to see this. > > > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java > <https://reviews.apache.org/r/5987/#comment22618> > > Thanks for getting rid of this, much cleaner! > > > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java > <https://reviews.apache.org/r/5987/#comment22619> > > Can you reformat this comment? > > /** > * Partition... > */ > > > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java > <https://reviews.apache.org/r/5987/#comment22620> > > This can be final right? > > > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java > <https://reviews.apache.org/r/5987/#comment22621> > > This is all handled by the send partition request I'm guessing? > > > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/DiskBackedPartitionStore.java > <https://reviews.apache.org/r/5987/#comment22623> > > MapMaker from guava is a bit more flexible perhaps. > > > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java > <https://reviews.apache.org/r/5987/#comment22622> > > You might want to use MapMaker(), from Guava here. > > > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/PartitionStore.java > <https://reviews.apache.org/r/5987/#comment22624> > > indent? > > > - Avery Ching > > > On Aug. 20, 2012, 2:04 p.m., Alessandro Presta wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/5987/ > > ----------------------------------------------------------- > > > > (Updated Aug. 20, 2012, 2:04 p.m.) > > > > > > Review request for giraph and Avery Ching. > > > > > > Description > > ------- > > > > I gave this another shot. This time it plays nicely with input > superstep: I replaced both the temporary partitions and the worker > partition map with a common data structure, PartitionStore, held in > ServerData. This is similar to what we now have for messages. > > > > Partition is now thread-safe so that we can have concurrent calls to > putVertex(). > > > > SimplePartitionStore is backed by a concurrent hash map (nothing new > here, except that we skip copying partitions to the worker). > > > > DiskBackedPartitionStore can hold up to a user-defined number of > partitions in memory, and spill the remaining ones to disk. Each partition > is stored in a separate file. > > Adding vertices to an out-of-core partition consists in appending them > to the file, which makes processing vertex requests relatively fast. > > We use a ReadWriteLock for each partition: performing operations on a > partition held in memory only requires a read-lock (since Partition is > thread-safe), while creating a new partition, moving it in and out of core > or appending vertices requires a write-lock (we can't have concurrent > writes). > > > > Also note that this breaks Hadoop RPC: I preferred to keep it clean > (this also shows what code we get rid of) instead of working around it. I > suppose the Netty security patch will be completed soon. If not, I will > restore RPC compatibility. > > > > More here: > https://issues.apache.org/jira/browse/GIRAPH-249?focusedCommentId=13435280&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13435280 > > > > > > This addresses bug GIRAPH-249. > > https://issues.apache.org/jira/browse/GIRAPH-249 > > > > > > Diffs > > ----- > > > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ServerData.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerServer.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/DiskBackedPartitionStore.javaPRE-CREATION > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/HashWorkerPartitioner.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/PartitionStore.javaPRE-CREATION > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/SimplePartitionStore.javaPRE-CREATION > > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/WorkerGraphPartitioner.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java1374990 > > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/graph/partition/TestPartitionStores.javaPRE-CREATION > > > > Diff: https://reviews.apache.org/r/5987/diff/ > > > > > > Testing > > ------- > > > > mvn verify and pseudo-distributed mode tests with both > SimplePartitionStore and DiskBackedPartitionStore > > > > > > Thanks, > > > > Alessandro Presta > > > > > >
