> On Aug. 21, 2012, 7:21 a.m., Avery Ching wrote: > > 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?
Sorry for that, it's the usual problem with renaming files. I could never get post-review to work with my checkout. Maja and I were thinking it would be helpful to have some guidelines on the website on how to create patches and (for committers) how to apply them (I use "patch -p0 -i" currently). For now, can you please manually rename TestMutateGraphVertex.java to TestMutateGraph.java after applying the patch? That should fix the build. Other than that, this passes mvn verify and pseudo-distributed mode tests with 4 tasks, both with RPC and Netty, with/without out-of-core enabled and with 1 or 2 max partitions in memory. You can find some benchmarks here (https://issues.apache.org/jira/browse/GIRAPH-249?focusedCommentId=13435280&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13435280) and in the following post. > On Aug. 21, 2012, 7:21 a.m., Avery Ching wrote: > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java, > > lines 1494-1497 > > <https://reviews.apache.org/r/5987/diff/4/?file=142956#file142956line1494> > > > > This is all handled by the send partition request I'm guessing? Yes, since doRequest directly adds to PartitionStore. - Alessandro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5987/#review10570 ----------------------------------------------------------- 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.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ServerData.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerServer.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/DiskBackedPartitionStore.java > PRE-CREATION > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/HashWorkerPartitioner.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/PartitionStore.java > PRE-CREATION > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/SimplePartitionStore.java > PRE-CREATION > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/WorkerGraphPartitioner.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestMutateGraphVertex.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java > 1374990 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/graph/partition/TestPartitionStores.java > PRE-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 > >
