I commented on Ticket 790 and created a new one: https://issues.apache.org/jira/browse/TINKERPOP-1058
-MP On Mon, Dec 21, 2015 at 8:13 AM, Mike Personick <[email protected]> wrote: > Thanks for the response Stephen. Yes I feel very strongly that any > open-ended result sets provided by the graph must be assumed to be backed > by a database resource that needs to be closed. It's true for Blazegraph > and it will certainly be true for others. When you ask for a tuple > iterator on a database you must close it, and that is what asking for an > iterator over edges, vertices, properties is. Materializing the result set > fully into memory first to manage this problem is not a viable solution for > a large graph. As I said, I hid a close() inside the last call to next(), > but this does not protect callers that do not fully drain the iterator. > > The EmptyVertexProperty and EmptyProperty is more of a nit - I need to > extend those to make certain tests in the StructureStandardSuite pass > without overriding them and changing their logic a bit. Those tests do > things like this: > > assertEquals(VertexProperty.empty(), p); > > Where in my case p is a BlazeVertexProperty (so that it can strengthen the > return type on any iterators to CloseableIterator), but the equality method > on EmptyVertexProperty checks the class of other. So I'd need to subclass > EmptyVertexProperty to make it work. Of course if we changed the GraphAPI > to use CloseableIterators then this change would become unnecessary as I > could use the "stock" API instead. > > We'll probably release/announce in early/mid January. I have the basics > working now but I'd like to dive in a little bit to optimizing the > implementation for traversals. I'd like to combine traversal steps and > execute them as SPARQL queries instead. Could you suggest a starting point > to look at for that? I have MatchStep on my list - what else should be > high on the list? > > Thanks, > Mike > > > > On Mon, Dec 21, 2015 at 6:08 AM, Stephen Mallette <[email protected]> > wrote: > >> Mike - we've often discussed AutoCloseable on iterators (among other >> things). To yell out a name, I believe Pieter Martin is in favor of that. >> We have a bit of a general discussion on the topic going here with the >> hope >> that we could make a change like that to get things straight for >> 3.2.0...please feel free to add your thoughts: >> >> https://issues.apache.org/jira/browse/TINKERPOP-790 >> >> > EmptyVertexProperty and EmptyProperty are declared final >> >> I'm not sure if we had a specific reason for finalizing those. We tend to >> finalize until someone yells and provides reasoning for removing the >> "final". Please add a ticket in JIRA and if no one objects, we could >> probably remove that modifier for 3.1.1. >> >> Thanks for the update btw - glad to hear that BlazeGraph is basically >> working under TP3 now. It would be nice to add BlazeGraph to the list of >> supporting graphs on the TinkerPop home page - will there be a general >> announcement of that anytime soon? >> >> >> >> On Sat, Dec 19, 2015 at 2:54 PM, Mike Personick <[email protected]> wrote: >> >> > Another much more minor problem is that EmptyVertexProperty and >> > EmptyProperty are declared final. Since the test suites specifically >> check >> > for instances of these classes, it makes it impossible for me to extend >> > them so that I can strengthen the return types on methods that return >> > VertexProperty and Property to Blaze-specific implementations of those >> > interfaces (ones that use CloseableIterator instead of Iterator). I'm >> just >> > going to have to skip a few tests in the test suite to get around this >> for >> > now. >> > >> > On Sat, Dec 19, 2015 at 10:51 AM, Mike Personick <[email protected]> >> wrote: >> > >> > > Still have a few kinds to work through but our basic Blazegraph / TP3 >> > > integration is more or less working at this point. >> > > >> > > Everything went pretty smoothly - my only major frustration with the >> API >> > > is the liberal use of Iterators, which are not AutoCloseable, and the >> > > built-in assumption widespread throughout the Tinkerpop codebase that >> > > iterations provided by the Graph implementation do not have any >> resource >> > > cleanup requirements. >> > > >> > > For example - Graph.edges() and Graph.vertices(). I am returning >> > > iterators backed by a database connection, which very much needs to be >> > > released when the iteration is done. I've done the best I can to >> protect >> > > the caller, by strengthening the return type to CloseableIterator >> > (extends >> > > Iterator and AutoCloseable), and I've even gone so far as to >> auto-close >> > the >> > > iterator inside next() if there is no hasNext(). But this does not >> fully >> > > protect the caller against resource leaks if they are writing to the >> > basic >> > > Graph API, (e.g. they do not fully drain the iterator). >> > > >> > > Humbly asking you to please take this into account in future version >> of >> > > the API. >> > > >> > > Thanks, >> > > Mike >> > > >> > >> > >
