Re: Review Request: GIRAPH-80 Refactor vertices to not expose the internal datastructure for holding messages
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3203/ --- (Updated 2011-12-17 09:36:06.229668) Review request for giraph. Summary (updated) --- refactoring that gives BasicVertex this 3 new methods: public abstract IterableM getMessages() returns an unmodifiable iterable allowing access to the current messages public abstract void setMessages(IterableM messages); replacement for getMsgList().clear() followed by getMsgList().addAll(...); public abstract void releaseResources(); after a vertex voted to halt, all references to messages it could still hold should be removed to enable earlier GC, instead of externally calling replacement for getMsgList().clear(), this method should be used Local unit tests pass, unfortunately I wasn't yet able to run the tests on my hadoop cluster (still have problems because I can only access it via a socks proxy) This addresses bug GIRAPH-80. https://issues.apache.org/jira/browse/GIRAPH-80 Diffs - /trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java 1215442 /trunk/src/main/java/org/apache/giraph/utils/ComparisonUtils.java PRE-CREATION /trunk/src/main/java/org/apache/giraph/utils/MemoryUtils.java 1215442 /trunk/src/test/java/org/apache/giraph/utils/ComparisonUtilsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/3203/diff Testing --- Thanks, Sebastian
[jira] [Updated] (GIRAPH-105) BspServiceMaster.checkWorkers() should return empty lists instead of null
[ https://issues.apache.org/jira/browse/GIRAPH-105?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sebastian Schelter updated GIRAPH-105: -- Attachment: GIRAPH-105.patch BspServiceMaster.checkWorkers() should return empty lists instead of null - Key: GIRAPH-105 URL: https://issues.apache.org/jira/browse/GIRAPH-105 Project: Giraph Issue Type: Bug Affects Versions: 0.70.0 Reporter: Sebastian Schelter Priority: Minor Attachments: GIRAPH-105.patch BspServiceMaster.checkWorkers() is invoked in BspServiceMaster.coordinateSuperstep() and in BspServiceMaster.createInputSplits(). Both check for an empty list to fail the job in case something has gone wrong. However, checkWorkers() returns null in case of problems, causing an NPE in the calling code. -- 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: Running tests in pseudo-distributed mode
We should document this somewhere. It is not intuitive as you mention. Avery On 12/17/11 1:41 AM, Sebastian Schelter wrote: A small hint for everyone who wants to run giraph's unit tests on a pseudo-distributed single node hadoop cluster: You have to adjust the configuration to allow 4 concurrent map tasks per node (default in hadoop-0.20.203 is 2), otherwise the tests will fail! You have to adjust mapred.tasktracker.map.tasks.maximum and mapred.map.tasks in mapred-site.xml. Took me a while to figure out :) --sebastian
Re: Running tests in pseudo-distributed mode
Wrote a blogpost about it :) Maybe it can be copied to giraph's wiki? http://ssc.io/running-giraphs-unit-tests-in-pseudo-distributed-mode/ --sebastian On 17.12.2011 18:38, Avery Ching wrote: We should document this somewhere. It is not intuitive as you mention. Avery On 12/17/11 1:41 AM, Sebastian Schelter wrote: A small hint for everyone who wants to run giraph's unit tests on a pseudo-distributed single node hadoop cluster: You have to adjust the configuration to allow 4 concurrent map tasks per node (default in hadoop-0.20.203 is 2), otherwise the tests will fail! You have to adjust mapred.tasktracker.map.tasks.maximum and mapred.map.tasks in mapred-site.xml. Took me a while to figure out :) --sebastian
Re: Running tests in pseudo-distributed mode
nice contribution, thanks! maybe you can write a small patch to the giraph documentation to add the content (and a link to your post, for your PR ;) )? On Sat, Dec 17, 2011 at 6:48 PM, Sebastian Schelter s...@apache.org wrote: Wrote a blogpost about it :) Maybe it can be copied to giraph's wiki? http://ssc.io/running-giraphs-unit-tests-in-pseudo-distributed-mode/ --sebastian On 17.12.2011 18:38, Avery Ching wrote: We should document this somewhere. It is not intuitive as you mention. Avery On 12/17/11 1:41 AM, Sebastian Schelter wrote: A small hint for everyone who wants to run giraph's unit tests on a pseudo-distributed single node hadoop cluster: You have to adjust the configuration to allow 4 concurrent map tasks per node (default in hadoop-0.20.203 is 2), otherwise the tests will fail! You have to adjust mapred.tasktracker.map.tasks.maximum and mapred.map.tasks in mapred-site.xml. Took me a while to figure out :) --sebastian -- Claudio Martella claudio.marte...@gmail.com
Re: Review Request: GIRAPH-80 Refactor vertices to not expose the internal datastructure for holding messages
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3203/#review3967 --- Ship it! +1. Thanks for the changes. I will commit and then open up a separate JIRA to make setMessages() package-private. - Avery On 2011-12-17 09:36:06, Sebastian Schelter wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3203/ --- (Updated 2011-12-17 09:36:06) Review request for giraph. Summary --- refactoring that gives BasicVertex this 3 new methods: public abstract IterableM getMessages() returns an unmodifiable iterable allowing access to the current messages public abstract void setMessages(IterableM messages); replacement for getMsgList().clear() followed by getMsgList().addAll(...); public abstract void releaseResources(); after a vertex voted to halt, all references to messages it could still hold should be removed to enable earlier GC, instead of externally calling replacement for getMsgList().clear(), this method should be used Local unit tests pass, unfortunately I wasn't yet able to run the tests on my hadoop cluster (still have problems because I can only access it via a socks proxy) This addresses bug GIRAPH-80. https://issues.apache.org/jira/browse/GIRAPH-80 Diffs - /trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1215442 /trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java 1215442 /trunk/src/main/java/org/apache/giraph/utils/ComparisonUtils.java PRE-CREATION /trunk/src/main/java/org/apache/giraph/utils/MemoryUtils.java 1215442 /trunk/src/test/java/org/apache/giraph/utils/ComparisonUtilsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/3203/diff Testing --- Thanks, Sebastian
[jira] [Commented] (GIRAPH-80) Don't expose the list holding the messages in BasicVertex
[ https://issues.apache.org/jira/browse/GIRAPH-80?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13171788#comment-13171788 ] jirapos...@reviews.apache.org commented on GIRAPH-80: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3203/#review3967 --- Ship it! +1. Thanks for the changes. I will commit and then open up a separate JIRA to make setMessages() package-private. - Avery On 2011-12-17 09:36:06, Sebastian Schelter wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/3203/ bq. --- bq. bq. (Updated 2011-12-17 09:36:06) bq. bq. bq. Review request for giraph. bq. bq. bq. Summary bq. --- bq. bq. refactoring that gives BasicVertex this 3 new methods: bq. bq. public abstract IterableM getMessages() bq. bq. returns an unmodifiable iterable allowing access to the current messages bq. bq. public abstract void setMessages(IterableM messages); bq. bq. replacement for getMsgList().clear() followed by getMsgList().addAll(...); bq. bq. public abstract void releaseResources(); bq. bq. after a vertex voted to halt, all references to messages it could still hold should be removed to enable earlier GC, instead of externally calling replacement for getMsgList().clear(), this method should be used bq. bq. Local unit tests pass, unfortunately I wasn't yet able to run the tests on my hadoop cluster (still have problems because I can only access it via a socks proxy) bq. bq. bq. This addresses bug GIRAPH-80. bq. https://issues.apache.org/jira/browse/GIRAPH-80 bq. bq. bq. Diffs bq. - bq. bq./trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1215442 bq./trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1215442 bq./trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java 1215442 bq./trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 1215442 bq./trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1215442 bq. /trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1215442 bq./trunk/src/main/java/org/apache/giraph/graph/Vertex.java 1215442 bq./trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java 1215442 bq./trunk/src/main/java/org/apache/giraph/utils/ComparisonUtils.java PRE-CREATION bq./trunk/src/main/java/org/apache/giraph/utils/MemoryUtils.java 1215442 bq./trunk/src/test/java/org/apache/giraph/utils/ComparisonUtilsTest.java PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/3203/diff bq. bq. bq. Testing bq. --- bq. bq. bq. Thanks, bq. bq. Sebastian bq. bq. Don't expose the list holding the messages in BasicVertex - Key: GIRAPH-80 URL: https://issues.apache.org/jira/browse/GIRAPH-80 Project: Giraph Issue Type: Improvement Affects Versions: 0.70.0 Reporter: Sebastian Schelter I'm currently trying to implement my own memory efficient vertex (similar to LongDoubleFloatDoubleVertex) and ran into problems with getMsgList() This method returns a list pointing to the messages of the vertex and it is modified externally (BasicRPCCommunications calls clear() and addAll() e.g.). This makes it very hard to use something else than a java.util.List internally (LongDoubleFloatDoubleVertex hacked around this) and it is generally dangerous to have the internal state of an object be modified externally. It also makes the code harder to read and understand. I'd suggest to change the API to let a vertex handle the modifications itself internally (e.g. add something like pushMessages(...)) -- 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] [Created] (GIRAPH-106) Refactor prepareSuperstep() to make setMessages(IterableM messages) package-private
Refactor prepareSuperstep() to make setMessages(IterableM messages) package-private - Key: GIRAPH-106 URL: https://issues.apache.org/jira/browse/GIRAPH-106 Project: Giraph Issue Type: Improvement Reporter: Avery Ching Assignee: Avery Ching GIRAPH-80 revealed that there is some refactoring to make setMessages() package-private (prevent users from messing around with internals). -- 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
