Review Request: GIRAPH-124: Combiner should return IterableM instead of M or null.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3592/ --- Review request for giraph. Summary --- Fixed the null check on the returned value and the javadoc.. Diffs - Diff: https://reviews.apache.org/r/3592/diff Testing --- local and MR unit tests Thanks, Claudio
[jira] [Commented] (GIRAPH-124) Combiner should return IterableM instead of M or null.
[ https://issues.apache.org/jira/browse/GIRAPH-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13191154#comment-13191154 ] Claudio Martella commented on GIRAPH-124: - Ok, fixed the javadoc and the null check. Yes, both unittests were run. See https://reviews.apache.org/r/3592/. Combiner should return IterableM 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 IterableM, 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
[jira] [Updated] (GIRAPH-45) Improve the way to keep outgoing messages
[ https://issues.apache.org/jira/browse/GIRAPH-45?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Claudio Martella updated GIRAPH-45: --- Attachment: GIRAPH-45.diff This a premature patch not meant for inclusion but as RFC. It passes all local unit tests and MR except checkpointing and partitioner tests. Apparently I broke something with partitioning. In case of checkpointing it breaks in BasicRPCCommunications#checkForMessageToNonExistentVertex(), with messages sent to the wrong worker (see IllegalStateException), while in TestGraphPartitioner the output partition files are small than required size. I'm requesting some comments as apparently I don't get how I broke partitioner package by moving some code from prepareSuperstep() to putMsg* methods. There must be an assumption I don't get which might be obvious to one of you. I tried to go incrementally by just refactoring BasicRPCCommunications#checkForMessageToNonExistentVertex() and leaving the rest AS-IS, so no out-of-core classes, just really trunk with BasicRPCCommunications#checkForMessageToNonExistentVertex() logics, and the code doesn't break. So... any ideas? Improve the way to keep outgoing messages - Key: GIRAPH-45 URL: https://issues.apache.org/jira/browse/GIRAPH-45 Project: Giraph Issue Type: Improvement Components: bsp Reporter: Hyunsik Choi Attachments: GIRAPH-45.diff As discussed in GIRAPH-12(http://goo.gl/CE32U), I think that there is a potential problem to cause out of memory when the rate of message generation is higher than the rate of message flush (or network bandwidth). To overcome this problem, we need more eager strategy for message flushing or some approach to spill messages into disk. The below link is Dmitriy's suggestion. https://issues.apache.org/jira/browse/GIRAPH-12?focusedCommentId=13116253page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13116253 -- 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
Review Request: GIRAPH-128: RPC port from BasicRPCCommunications should be only a starting port, and retried
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3596/ --- Review request for giraph. Summary --- Simple handling of port collisions on the same machine while preserving debugability from the port number alone. Round up the max number of workers to the next power of 10 and use it as a constant to increase the port number with. Added a unit test to ensure it is working correctly. Fixed 2 minor warnings on src/main/java/org/apache/giraph/examples/MinimumIntCombiner.java src/main/java/org/apache/giraph/examples/SimpleSumCombiner.java of removing 'import java.util.List'. This addresses bug GIRAPH-128. https://issues.apache.org/jira/browse/GIRAPH-128 Diffs - http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1234970 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/MinimumIntCombiner.java 1234970 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSumCombiner.java 1234970 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/comm/RPCCommunicationsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/3596/diff Testing --- Passed local and MR unittests. Thanks, Avery
[jira] [Commented] (GIRAPH-128) RPC port from BasicRPCCommunications should be only a starting port, and retried
[ https://issues.apache.org/jira/browse/GIRAPH-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13191425#comment-13191425 ] jirapos...@reviews.apache.org commented on GIRAPH-128: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3596/ --- Review request for giraph. Summary --- Simple handling of port collisions on the same machine while preserving debugability from the port number alone. Round up the max number of workers to the next power of 10 and use it as a constant to increase the port number with. Added a unit test to ensure it is working correctly. Fixed 2 minor warnings on src/main/java/org/apache/giraph/examples/MinimumIntCombiner.java src/main/java/org/apache/giraph/examples/SimpleSumCombiner.java of removing 'import java.util.List'. This addresses bug GIRAPH-128. https://issues.apache.org/jira/browse/GIRAPH-128 Diffs - http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1234970 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/MinimumIntCombiner.java 1234970 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSumCombiner.java 1234970 http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/comm/RPCCommunicationsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/3596/diff Testing --- Passed local and MR unittests. Thanks, Avery RPC port from BasicRPCCommunications should be only a starting port, and retried Key: GIRAPH-128 URL: https://issues.apache.org/jira/browse/GIRAPH-128 Project: Giraph Issue Type: Improvement Affects Versions: 0.1.0 Reporter: Avery Ching Assignee: Avery Ching Currently Giraph uses a basic port + the task partition to get the RPC port. This doesn't work well for when there are multiple Giraph jobs running simultaneously in the same Hadoop cluster (port conflict). At the same time, it is nice to use this simple algorithm because it makes it very easy to debug problems (you can find the troublesome mapper from the RPC port name). I will be proposing a simple scheme to retry with another port. I will round the total number of mappers up to the nearest power of 10 (let's that that number Z). Then I will increment the port number by Z, retrying up to 20 tries. If you have enough ports, this scheme would guarantee that up to 20 mappers / node would be supported. It should be sufficient for most clusters. At the same time, we still maintain the easy debugging method since you it's still easy to figure out the mapper partition from the port (port % Z = map partition). -- 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
[jira] [Commented] (GIRAPH-124) Combiner should return IterableM instead of M or null.
[ https://issues.apache.org/jira/browse/GIRAPH-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13191431#comment-13191431 ] Claudio Martella commented on GIRAPH-124: - damn RB. it should be fixed now... Combiner should return IterableM 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 IterableM, 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
Re: Review Request: GIRAPH-124: Combiner should return IterableM instead of M or null.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3592/#review4538 --- Ship it! Looks great! One very minor thing. Before you commit, can you please prefix the exception messages with run: ? The other exception messages always start with the method name and then a colon. You don't need to resubmit a review, just make those changes and please commit. Thanks! trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java https://reviews.apache.org/r/3592/#comment10130 please prefix with run: trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java https://reviews.apache.org/r/3592/#comment10133 please prefix with run: trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java https://reviews.apache.org/r/3592/#comment10131 please prefix with run: trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java https://reviews.apache.org/r/3592/#comment10132 please prefix with run: - Avery On 2012-01-23 20:38:48, Claudio Martella wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3592/ --- (Updated 2012-01-23 20:38:48) Review request for giraph. Summary --- Fixed the null check on the returned value and the javadoc.. Diffs - trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1234376 trunk/src/main/java/org/apache/giraph/examples/MinimumIntCombiner.java 1234376 trunk/src/main/java/org/apache/giraph/examples/SimpleSumCombiner.java 1234376 trunk/src/main/java/org/apache/giraph/graph/VertexCombiner.java 1234376 trunk/src/test/java/org/apache/giraph/TestVertexTypes.java 1234376 trunk/src/test/java/org/apache/giraph/examples/MinimumIntCombinerTest.java 1234376 Diff: https://reviews.apache.org/r/3592/diff Testing --- local and MR unit tests Thanks, Claudio
[jira] [Commented] (GIRAPH-124) Combiner should return IterableM instead of M or null.
[ https://issues.apache.org/jira/browse/GIRAPH-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13191480#comment-13191480 ] Claudio Martella commented on GIRAPH-124: - Commited, thanks. Combiner should return IterableM 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 IterableM, 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
[jira] [Commented] (GIRAPH-124) Combiner should return IterableM instead of M or null.
[ https://issues.apache.org/jira/browse/GIRAPH-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13191510#comment-13191510 ] Hudson commented on GIRAPH-124: --- Integrated in Giraph-trunk-Commit #65 (See [https://builds.apache.org/job/Giraph-trunk-Commit/65/]) GIRAPH-124: Combiner should return IterableM instead of M or null. claudio : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVNview=revrev=1234997 Files : * /incubator/giraph/trunk/CHANGELOG * /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/MinimumIntCombiner.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSumCombiner.java * /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexCombiner.java * /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexTypes.java * /incubator/giraph/trunk/src/test/java/org/apache/giraph/examples/MinimumIntCombinerTest.java Combiner should return IterableM 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 IterableM, 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
[jira] [Resolved] (GIRAPH-124) Combiner should return IterableM instead of M or null.
[ https://issues.apache.org/jira/browse/GIRAPH-124?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Claudio Martella resolved GIRAPH-124. - Resolution: Fixed Combiner should return IterableM 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 IterableM, 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
