Hi avery,
sorry forgot resolver was exported to user space. I ll consider this. About
your idea, it makes sense although I somehow I believe that if user space
messes up it s not our fault. Your solution though makes evrrybody happy.
Will implement this and send the separate patch. Thanks

On Friday, January 6, 2012, Avery Ching <ach...@apache.org> wrote:
> Hi Claudio, answers inline:
> On 1/6/12 8:25 AM, Claudio Martella wrote:
>> Hello,
>> I hope somebody can shed some light on a piece of code i'm looking at
>> while working on GIRAPH-45 (and this code is also the object of
>> GIRAPH-95, so we'd probably get two birds with one stone here).
>> The code is taking care of vertex resolving in
>> BasicRPCCommunication::prepareSuperstep():
>> [line 1091]:
>>            if (vertex != null) {
>>                 ((MutableVertex<I, V, E, M>)
>>                 partition.putVertex((BasicVertex<I, V, E, M>) vertex);
>>             } else if (originalVertex != null) {
>>                 partition.removeVertex(originalVertex.getVertexId());
>>             }
>> First, vertex cannot be null as it's resolved by vertexRevolver, but i
>> guess it's a sanity check. But the real question is: why would you
>> setVertex() considering it's been already initialized correctly in
>> vertexResolver?
> Actually it can be null.  Since user's can implement their own vertex
resolver, they are allowed to return null from the javadoc.
>    /**
>     * A vertex may have been removed, created zero or more times and had
>     * zero or more messages sent to it.  This method will handle all
>     * excluding the normal case (a vertex already exists and has zero or
>     * messages sent it to).
>     *
>     * @param vertexId Vertex id (can be used for {@link BasicVertex}'s
>     *        initialize())
>     * @param vertex Original vertex or null if none
>     * @param vertexChanges Changes that happened to this vertex or null
if none
>     * @param messages messages received in the last superstep or null if
>     * @return Vertex to be returned, if null, and a vertex currently
>     *         it will be removed
>     */
>> Am I missing something or did I just realize that GIRAPH-95 is solved
>> by just removing that line? :)
>> Thanks
> Well, not sure about that.  The set is done there I think to ensure
safety.  Here's the issue:  Suppose that the resolve() doesn't set the
vertex id correctly (i.e. in this partition).  That would be a bug and
probably cause issues.  Probably this should be changed to be a check
though.  Something like...
>        if (vertex != null) {
>            if (vertex.getVertexId().equals(vertexIndex)) {
>                throw new IllegalStateException("BasicRPCCommunications:
Illegal to set the vertex index differently from " + vertexIndex);
>            if (originalVertex == null) {
>                partition.putVertex((BasicVertex<I, V, E, M>) vertex);
>            } else {
>                partition.removeVertex(originalVertex.getVertexId());
>            }
>        }
> What do you think?
> Avery

   Claudio Martella

Reply via email to