incubator-giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Avery Ching (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (GIRAPH-124) Combiner should return Iterable<M> instead of M or null.
Date Sun, 22 Jan 2012 08:07:38 GMT

    [ https://issues.apache.org/jira/browse/GIRAPH-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13190625#comment-13190625
] 

Avery Ching commented on GIRAPH-124:
------------------------------------

Thanks for the improvements, I have a few more comments.  These comments are a little tricky
on JIRA, do you want to try reviewboard next time?

{noformat} 
+   /**
+    * Combines message values for a particular vertex index.
+    *
+    * @param vertexIndex Index of the vertex getting these messages
+    * @param messages Iterable of the messages to be combined
+    * @return Message that is combined from {@link messages} or null if no
+    *         message it to be sent
+    * @throws IOException
+    */
+    public abstract Iterable<M> combine(I vertexIndex,
+            Iterable<M> messages) throws IOException;
{noformat} 

This javadoc is not quite right.  @return should be corrected, for instance, null is not legal
now right?

{noformat}
+                        Iterable<M> messages = combiner.combine(entry.getKey(),
                                                          entry.getValue());
{noformat}

Can you align 'entry.getValue()'?

{noformat}
+                    if (Iterables.size(outMessageList) < 
+                            Iterables.size(messages)) {
{noformat}

Before you do this check, we still need to do a check that messages is not null.  If it is
null, the combiner has violated the combine() contract.  We need to throw an exception at
this point.

Also, have you run the unittests (local and MR)?

Thanks!

                
> Combiner should return Iterable<M> instead of M or null.
> --------------------------------------------------------
>
>                 Key: GIRAPH-124
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-124
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.1.0
>            Reporter: Claudio Martella
>         Attachments: GIRAPH-124.diff, GIRAPH-124.diff
>
>
> Currently VertexCombiner is expected to return a single message combining the input messages,
or null in case no message should be sent. The new expected interface should return an Iterable<M>,
possibly empty. The number of elements in the returned Iterable is supposed to be smaller
than the number of input messages, by the initial definition of a Combiner (defined as a function
to reduce I/O by combining multiple messages into 1).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message