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

Jakob Homan commented on GIRAPH-36:
-----------------------------------

Great work, Jake. One thing I notice while reading through Vertex is muddling 
of the state the Vertex is responsible for on a per-application basis and the 
state Giraph manages for the vertex.  I think we may being ill-served by 
inheritance here and should instead rely on composition to hold this state.  
I'm thinking that messages in/out, edge state and mutation, facilities for 
sending messages, current superstep, etc. Would it be better in the long term 
to move these out to a context object (ala MR)?  This would simplify Vertex 
significantly, make it much easier to test (by mocking out the dependency) and 
future proof the evolution of Vertex as there will be fewer moving parts to 
keep track of.

Does changing compute and {pre|post}{Superstep|Application} took their external 
state as parameters with sound like a good approach to explore?
                
> Ensure that subclassing BasicVertex is possible by user apps
> ------------------------------------------------------------
>
>                 Key: GIRAPH-36
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-36
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>            Priority: Blocker
>             Fix For: 0.70.0
>
>         Attachments: GIRAPH-36.diff, GIRAPH-36.diff, GIRAPH-36.diff, 
> GIRAPH-36.diff.warnings
>
>
> Original assumptions in Giraph were that all users would subclass Vertex 
> (which extended MutableVertex extended BasicVertex).  Classes which wish to 
> have application specific data structures (ie. not a TreeMap<I, Edge<I,E>>) 
> may need to extend either MutableVertex or BasicVertex.  Unfortunately 
> VertexRange extends ArrayList<Vertex>, and there are other places where the 
> assumption is that vertex classes are either Vertex, or at least 
> MutableVertex.
> Let's make sure the internal APIs allow for BasicVertex to be the base class.

--
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

        

Reply via email to