Return-Path: X-Original-To: apmail-giraph-dev-archive@www.apache.org Delivered-To: apmail-giraph-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id DDFC6D917 for ; Tue, 21 Aug 2012 07:21:49 +0000 (UTC) Received: (qmail 14886 invoked by uid 500); 21 Aug 2012 07:21:49 -0000 Delivered-To: apmail-giraph-dev-archive@giraph.apache.org Received: (qmail 14738 invoked by uid 500); 21 Aug 2012 07:21:49 -0000 Mailing-List: contact dev-help@giraph.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@giraph.apache.org Delivered-To: mailing list dev@giraph.apache.org Delivered-To: moderator for dev@giraph.apache.org Received: (qmail 12815 invoked by uid 99); 21 Aug 2012 07:21:24 -0000 Content-Type: multipart/alternative; boundary="===============8196200937300618043==" MIME-Version: 1.0 Subject: Re: Review Request: Move part of the graph out-of-core when memory is low From: "Avery Ching" To: "Avery Ching" Cc: "giraph" , "Alessandro Presta" Date: Tue, 21 Aug 2012 07:21:23 -0000 Message-ID: <20120821072123.15255.94874@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Avery Ching" X-ReviewGroup: giraph X-ReviewRequest-URL: https://reviews.apache.org/r/5987/ X-Sender: "Avery Ching" References: <20120820140420.15255.74960@reviews.apache.org> In-Reply-To: <20120820140420.15255.74960@reviews.apache.org> Reply-To: "Avery Ching" --===============8196200937300618043== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- 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 c= odebase. 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/apach= e/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/girap= h/comm/BasicRPCCommunications.java This is way cleaner. Definitely pleased to see this. http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/girap= h/comm/BasicRPCCommunications.java Thanks for getting rid of this, much cleaner! http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/girap= h/graph/BspServiceWorker.java Can you reformat this comment? = /** * Partition... */ http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/girap= h/graph/BspServiceWorker.java This can be final right? http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/girap= h/graph/BspServiceWorker.java 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/girap= h/graph/partition/DiskBackedPartitionStore.java MapMaker from guava is a bit more flexible perhaps. http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/girap= h/graph/partition/Partition.java You might want to use MapMaker(), from Guava here. http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/girap= h/graph/partition/PartitionStore.java 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 simil= ar to what we now have for messages. > = > Partition is now thread-safe so that we can have concurrent calls to putV= ertex(). > = > 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 partitio= ns in memory, and spill the remaining ones to disk. Each partition is store= d 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 par= tition 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 appe= nding 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 suppos= e the Netty security patch will be completed soon. If not, I will restore R= PC compatibility. > = > More here: https://issues.apache.org/jira/browse/GIRAPH-249?focusedCommen= tId=3D13435280&page=3Dcom.atlassian.jira.plugin.system.issuetabpanels:comme= nt-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/g= iraph/bsp/CentralizedServiceWorker.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/comm/BasicRPCCommunications.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/comm/NettyWorkerClientServer.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/comm/NettyWorkerServer.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/comm/SendVertexRequest.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/comm/ServerData.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/comm/WorkerServer.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/graph/BspServiceWorker.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/graph/GiraphJob.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/graph/GraphMapper.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/graph/partition/DiskBackedPartitionStore.java PRE-CREATION = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/graph/partition/HashWorkerPartitioner.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/graph/partition/Partition.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/graph/partition/PartitionStore.java PRE-CREATION = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/graph/partition/SimplePartitionStore.java PRE-CREATION = > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/g= iraph/graph/partition/WorkerGraphPartitioner.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/g= iraph/TestMutateGraphVertex.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/g= iraph/comm/ConnectionTest.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/g= iraph/comm/RequestTest.java 1374990 = > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/g= iraph/graph/partition/TestPartitionStores.java PRE-CREATION = > = > Diff: https://reviews.apache.org/r/5987/diff/ > = > = > Testing > ------- > = > mvn verify and pseudo-distributed mode tests with both SimplePartitionSto= re and DiskBackedPartitionStore > = > = > Thanks, > = > Alessandro Presta > = > --===============8196200937300618043==--