giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alessandro Presta (JIRA)" <>
Subject [jira] [Commented] (GIRAPH-270) Improve TestManualCheckpoint
Date Wed, 08 Aug 2012 16:49:20 GMT


Alessandro Presta commented on GIRAPH-270:

Good start!

TestCheckpoints looks clean, but what about a comment explaining what it tests (I see an empty
javadoc)? E.g. "Tests correctness of reloading from a checkpoint." Also, maybe you can rename
it to something more explicit like TestLoadCheckpoint.

What looks fragile is how SimpleIncrementVertex depends on the particular graph which is defined
in TestCheckpoints.
A way to fix this would be to make the vertex a static inner class of the test case, to make
the dependency more evident.

Regarding the vertex itself:
- the checks are a bit obscure, and I don't like the call to System.exit(). I would break
them down so that for each one you can explain what it checks (e.g. "checks the vertex value")
and throw an informative exception. If you make it an inner class as I suggested, you can
also use assertions from JUnit which are way cleaner.
- the javadoc here is a little confusing, I would go for something more concise. Better to
explain the algorithm and the various checks as inline comments than all at once.
- maybe you should also check that the number of edges and messages is 1?

Are you still going to fix TestManualCheckpoint? Since that relies on aggregators, we can
block this on GIRAPH-259 which hopefully will be committed in a matter of days. You can add
the fix to this patch (it will require a little rebase when GIRAPH-259 goes in).

> Improve TestManualCheckpoint
> ----------------------------
>                 Key: GIRAPH-270
>                 URL:
>             Project: Giraph
>          Issue Type: Bug
>            Reporter: Alessandro Presta
>            Assignee: Aravind Anbudurai
>         Attachments: Improve_TestManualCheckpoint.patch
> A good test case for checkpointing is important as we experiment, for example, with storing
messages separately to enable out-of-core messaging.
> There are two problems with TestManualCheckpoint:
> - It's only checking that the sum of vertex ids is invariant. This doesn't guarantee
that we're correctly restoring vertex values, edges, and messages.
> - The "restarted job" seems to actually start from scratch, and overwrite the previous
job's checkpoints. What we are expecting, instead, is the restarted job to load the latest
checkpoint and start from there.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:!default.jspa
For more information on JIRA, see:


View raw message