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
> 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:
For more information on JIRA, see: http://www.atlassian.com/software/jira