Giraph
  1. Giraph
  2. GIRAPH-118

Clarify messages behavior in BasicVertex

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.1.0
    • Fix Version/s: 0.1.0
    • Component/s: graph
    • Labels:
      None

      Description

      initialize() can receive a null parameter for messages (at least that's what EdgeListVertex does). We should avoid that and pass an empty Iterable instead. That should be cheap for us inside of the InputFormat, just passing a static immutable empty list.

      setMessages(Iterable<M>) should be changed to putMessages(Iterable<M>). the set prefix suggests an assignment, while setMessages is used to transfer the messages to the internal datastructure the user is responsible for. putMessages() should clarify this.

      1. GIRAPH-118.diff
        4 kB
        Claudio Martella
      2. GIRAPH-119.diff
        4 kB
        Claudio Martella

        Activity

        Claudio Martella created issue -
        Claudio Martella made changes -
        Field Original Value New Value
        Assignee Claudio Martella [ cmartella ]
        Affects Version/s 0.70.0 [ 12317572 ]
        Description initialize() can receive a null parameter for messages (at least that's what EdgeListVertex does). We should avoid that and pass an empty Iterable instead. That should be cheap for us inside of the InputFormat, just passing a static immutable empty list.

        setMessages(Iterable<M>) should be changed to putMessages(Iterable<M>). the set prefix suggests an assignment, while setMessages is used to transfer the messages to the internal datastructure the user is responsible for. putMessages() should clarify this.
        Hide
        Claudio Martella added a comment -

        As a user with some experience with Giraph, it took a while to rationalize the process while working on GIRAPH-45, I guess a new user should be a bit confused as well.

        Show
        Claudio Martella added a comment - As a user with some experience with Giraph, it took a while to rationalize the process while working on GIRAPH-45 , I guess a new user should be a bit confused as well.
        Hide
        Avery Ching added a comment -

        Seems reasonable. Please make sure to update the javadoc and the MutableVertex implementations.

        Show
        Avery Ching added a comment - Seems reasonable. Please make sure to update the javadoc and the MutableVertex implementations.
        Hide
        Claudio Martella added a comment -

        Apparently the initialize() issue is also true for other parameters as well such as the edges (and outside of the documentation also vertexId: I'm looking at TestEdgeListVertex i.e.).

        With this little one I just touched the putMessages() issue, probably we can think about the initialize() later.

        Show
        Claudio Martella added a comment - Apparently the initialize() issue is also true for other parameters as well such as the edges (and outside of the documentation also vertexId: I'm looking at TestEdgeListVertex i.e.). With this little one I just touched the putMessages() issue, probably we can think about the initialize() later.
        Claudio Martella made changes -
        Attachment GIRAPH-119.diff [ 12509741 ]
        Hide
        Claudio Martella added a comment -

        Messed up with issue number in patch filename, sorry

        Show
        Claudio Martella added a comment - Messed up with issue number in patch filename, sorry
        Claudio Martella made changes -
        Attachment GIRAPH-118.diff [ 12509742 ]
        Hide
        Avery Ching added a comment -

        +1, looks good!

        Show
        Avery Ching added a comment - +1, looks good!
        Hide
        Claudio Martella added a comment -

        Thanks, commited. For the record, I also corrected the javadoc of BasicVertex::releaseResources() stating the method is called after the vertex calls voteToHalt() but is called after vertex returns from compute().

        Show
        Claudio Martella added a comment - Thanks, commited. For the record, I also corrected the javadoc of BasicVertex::releaseResources() stating the method is called after the vertex calls voteToHalt() but is called after vertex returns from compute().
        Claudio Martella made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        Integrated in Giraph-trunk-Commit #62 (See https://builds.apache.org/job/Giraph-trunk-Commit/62/)
        GIRAPH-118: Clarify messages behavior in BasicVertex

        claudio : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1228691
        Files :

        • /incubator/giraph/trunk/CHANGELOG
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/HashMapVertex.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java
        • /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java
        Show
        Hudson added a comment - Integrated in Giraph-trunk-Commit #62 (See https://builds.apache.org/job/Giraph-trunk-Commit/62/ ) GIRAPH-118 : Clarify messages behavior in BasicVertex claudio : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1228691 Files : /incubator/giraph/trunk/CHANGELOG /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/HashMapVertex.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java
        Jakob Homan made changes -
        Fix Version/s 0.1.0 [ 12317572 ]

          People

          • Assignee:
            Claudio Martella
            Reporter:
            Claudio Martella
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development