incubator-giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jake Mannix (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (GIRAPH-36) Ensure that subclassing BasicVertex is possible by user apps
Date Sat, 29 Oct 2011 03:37:32 GMT

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

Jake Mannix commented on GIRAPH-36:
-----------------------------------

Ok, I'm digging into this again, finally... man patches get pretty stale if left unattended
in the Incubator. :)

So, the idea with keeping BasicVertex which is *not* mutable is not for performance, but for
implementation ease / coding safety: forcing users to implement mutating methods on their
graph which doesn't allow such things is unfair to the users of this library, and having them
throw UnsupportedOperationException should be avoided because then lots of problems are only
seen at runtime, and then when the project moves forward and adds some innocuous thing which
passes all unit tests, but ends up calling some mutator method that wasn't called before (maybe
as part of a "post-initialization check of correctness, or something"), the users who are
throwing exceptions in these methods only find out when they run their jobs which used to
succeed, and now die with mysterious UOE getting thrown in new methods nowhere near their
code.  Bad news, if it can be avoided.

So I'm in favor of a) over b), in theory.  It's closest to the way things are done in hadoop,
and seems the cleanest, from a purity of API standpoint.

However, having dug into the code a bit, it's going to be hairy to get a) to work easily,
now that there is a proliferation of VertexInputFormat stuff that Jakob has been committing
lately (yay! but boo. :( ) - all of this code would also need to be retrofitted to match.

So I'm going to take a stab at b).  It breaks the API least, and while it appears to allow
mutation, it's pretty clear to the user that it's not really, and we can forcibly throw exceptions
on multiple use, for example, to keep it from being reused.
                
> 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
>
>
> 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

        

Mime
View raw message