----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23140/#review47838 -----------------------------------------------------------
Looks great, a few final comments about the test. giraph-examples/src/test/java/org/apache/giraph/TestCheckpointing.java <https://reviews.apache.org/r/23140/#comment84071> I'm a bit concerned that this test would have passed even if restart from checkpoint didn't actually happen but app run from beginning. Can we somehow ensure it did? giraph-examples/src/test/java/org/apache/giraph/TestCheckpointing.java <https://reviews.apache.org/r/23140/#comment84066> Can you reuse the same conf and just add one setting (or at least create a method which creates conf) giraph-examples/src/test/java/org/apache/giraph/TestCheckpointing.java <https://reviews.apache.org/r/23140/#comment84068> You can extend DefaultWorkerContext to avoid overriding empty methods - Maja Kabiljo On July 15, 2014, 11:33 p.m., Sergey Edunov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23140/ > ----------------------------------------------------------- > > (Updated July 15, 2014, 11:33 p.m.) > > > Review request for giraph. > > > Repository: giraph-git > > > Description > ------- > > This fix merely makes checkpointing work again. > > > Diffs > ----- > > > giraph-core/src/main/java/org/apache/giraph/aggregators/AggregatorWrapper.java > 9613805 > giraph-core/src/main/java/org/apache/giraph/bsp/BspService.java 2e35373 > giraph-core/src/main/java/org/apache/giraph/comm/ServerData.java 85bfe04 > giraph-core/src/main/java/org/apache/giraph/conf/GiraphConstants.java > ab0570f > giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java > 0275395 > > giraph-core/src/main/java/org/apache/giraph/partition/BasicPartitionOwner.java > 545d1af > > giraph-core/src/main/java/org/apache/giraph/partition/HashMasterPartitioner.java > 240687e > > giraph-core/src/main/java/org/apache/giraph/partition/HashWorkerPartitioner.java > d833895 > > giraph-core/src/main/java/org/apache/giraph/partition/MasterGraphPartitioner.java > 50c750a > > giraph-core/src/main/java/org/apache/giraph/partition/PartitionBalancer.java > 3454d62 > giraph-core/src/main/java/org/apache/giraph/partition/PartitionOwner.java > 0ac74da > > giraph-core/src/main/java/org/apache/giraph/partition/SimpleMasterPartitioner.java > f128f34 > > giraph-core/src/main/java/org/apache/giraph/partition/SimpleWorkerPartitioner.java > 3c0de44 > > giraph-core/src/main/java/org/apache/giraph/partition/WorkerGraphPartitioner.java > 004ea81 > giraph-core/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java > 2c4606f > > giraph-core/src/main/java/org/apache/giraph/utils/io/ExtendedDataInputOutput.java > af45426 > giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java > de7af28 > giraph-core/src/main/java/org/apache/giraph/worker/WorkerContext.java > 29835c5 > > giraph-core/src/test/java/org/apache/giraph/partition/SimpleRangePartitionFactoryTest.java > 96bd5d7 > giraph-examples/src/test/java/org/apache/giraph/TestCheckpointing.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/23140/diff/ > > > Testing > ------- > > I tested it running multiple different jobs. I run page rank on 2*10^9 > vertices on 200 workers and it seems to work just fine. It only takes 2 > minutes to save checkpoint. > > > Thanks, > > Sergey Edunov > >
