You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by Stephen Mallette <sp...@gmail.com> on 2015/10/05 21:56:41 UTC

[DISCUSS] Semantics of close()

There are two tickets in JIRA that relate to the semantics of closing and
releasing resources in the Graph hierarchy:

+ Graph
+ TraversalSource
+ TraversalStrategy
+ GraphTraversal
+ Transaction

https://issues.apache.org/jira/browse/TINKERPOP3-789
https://issues.apache.org/jira/browse/TINKERPOP3-790

There's been some discussion on them already.  I bring this to everyone's
attention as the change could have wide repercussions depending on the
direction it goes.  The short of the matter is that currently:

1. Graph implements AutoCloseable, but we don't enforce the notion of
close() itself in the test suite.
2. TraversalSource should likely implement AutoCloseable as there are
sometimes resources that need to be released when a TraversalSource is no
longer in use.
3. Transaction implements AutoCloseable but it applies in the context of
how a  Transaction will behave when Graph.close() is called.

Where is this all going?  Matt Frantz summarized the thought points nicely
in this comment:

https://issues.apache.org/jira/browse/TINKERPOP3-790?focusedCommentId=14710389&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14710389

Basically, there's two options Matt identified:

1. Making it so that Graph.close() would release resources and effectively
close() all things it spawned is one approach, but I''m not sure how
straightforward/practical to implement that is.  I suspect it will
complicate Graph implementations as well.
2. The alternative is that the things in this hierarchy can have their own
expensive resources which can be independently closed.  I think that this
approach more closely fits with the code as we have it now and won't unduly
burden implementers.  TinkerPop Developers will have to know when to call
close() (e.g. if a TraversalStrategy has expensive resources and the user
assumes that a call to TraversalSource.close() will release those
resources, then they might be wrong - depends on the approach we decide
on).

When I first created 790, i had the second option in mind, but since Matt
brought wrote that comment, i figured it was worth thinking through as a
whole.

Once that is decided then we should figure out 789 which is how to enforce
the semantics of close() (e.g. what does a Graph do when it's closed and
someone calls graph.traversal()?).

Thoughts?

Re: [DISCUSS] Semantics of close()

Posted by Stephen Mallette <sp...@gmail.com>.
I left this issue alone for a couple of weeks as 3.0.2 was looming.  Here's
my summary of where the discussion went:

1. Bryn liked the idea of maintaining a strong hierarchy for close()
2. I mentioned that the strong hierarchy is intuitive, but would place
burden on providers and complicate code considerably.
3. Pieter Martin explained what sqlg does on close() which immediately
introduced a break to a strong hierarchy (a pattern I think we would see
across providers in various ways).

I think that we should move in favor of implementing AutoCloseable where it
makes sense to release resources and not force a strict hierarchy.  This
places some burden on users to know how/when to close things, but if we
document carefully we can mitigate that issue.

If no-one objects within 72 hours, I'll assume lazy consensus and move in
that fashion to get those JIRA tickets closed for 3.1.0.



On Wed, Oct 7, 2015 at 8:57 AM, Stephen Mallette <sp...@gmail.com>
wrote:

> I'm fine with all items in the Graph hierarchy implementing AutoCloseable
> (including GraphTraversal).  I'm not sure what the implementation would
> look like though and what it would do.  Perhaps I'll take a look at
> ResourceIterator from Neo4j.  If we go with the idea that the user is
> responsible for properly closing things and close() on each component works
> in this independent fashion, then we can fire up a separate discussion for
> what close() on GraphTraversal means (and each item in the Graph
> hierarchy).  Sounds like you're in favor of this approach - i guess we'll
> see if anyone else has other opinions.
>
> On Wed, Oct 7, 2015 at 8:34 AM, pieter-gmail <pi...@gmail.com>
> wrote:
>
>> Ok, how about making GraphTraversal implement AutoCloseable?
>>
>> Thanks
>> Pieter
>>
>> On 07/10/2015 14:28, Stephen Mallette wrote:
>> > n I think the best approach is to leave it to users to close()
>> > resources properly and keep document it as a convention.  So Option 2
>> from
>> > my initial post in this
>>
>>
>

Re: [DISCUSS] Semantics of close()

Posted by Stephen Mallette <sp...@gmail.com>.
I'm fine with all items in the Graph hierarchy implementing AutoCloseable
(including GraphTraversal).  I'm not sure what the implementation would
look like though and what it would do.  Perhaps I'll take a look at
ResourceIterator from Neo4j.  If we go with the idea that the user is
responsible for properly closing things and close() on each component works
in this independent fashion, then we can fire up a separate discussion for
what close() on GraphTraversal means (and each item in the Graph
hierarchy).  Sounds like you're in favor of this approach - i guess we'll
see if anyone else has other opinions.

On Wed, Oct 7, 2015 at 8:34 AM, pieter-gmail <pi...@gmail.com>
wrote:

> Ok, how about making GraphTraversal implement AutoCloseable?
>
> Thanks
> Pieter
>
> On 07/10/2015 14:28, Stephen Mallette wrote:
> > n I think the best approach is to leave it to users to close()
> > resources properly and keep document it as a convention.  So Option 2
> from
> > my initial post in this
>
>

Re: [DISCUSS] Semantics of close()

Posted by pieter-gmail <pi...@gmail.com>.
Ok, how about making GraphTraversal implement AutoCloseable?

Thanks
Pieter

On 07/10/2015 14:28, Stephen Mallette wrote:
> n I think the best approach is to leave it to users to close()
> resources properly and keep document it as a convention.  So Option 2 from
> my initial post in this 


Re: [DISCUSS] Semantics of close()

Posted by Stephen Mallette <sp...@gmail.com>.
> In sqlg I close the connection pool to the db immediately. No waiting around
for transactions to complete.

It is my sense that this approach is what most Graph implementations do.
You Graph.close() then everything else that was spawned from Graph reacts
to the close however it will.  It is up to the user to manage Graph.close()
in such a way that all other objects that use Graph have close() called on
them first.  It sounds like Neo4j goes that route too.

I think we will find wide disparity in close() procedures for the various
Graph implementations and if we try to enforce a hierarchical close() we
will complicate a lot of code and in the end not get a nice clean
abstraction that applies generally across the board.  If that conclusion is
true, then I think the best approach is to leave it to users to close()
resources properly and keep document it as a convention.  So Option 2 from
my initial post in this thread.



On Wed, Oct 7, 2015 at 8:18 AM, pieter-gmail <pi...@gmail.com>
wrote:

> No I don't know. Their GraphDatabaseService.shutdown() javadoc says,
>
> "Shuts down Neo4j. After this method has been invoked, it's invalid to
> invoke any methods in the Neo4j API and all references to this instance
> of GraphDatabaseService should be discarded."
>
> That sorta means that they will clean up all dangling resources implicitly.
>
> In sqlg I close the connection pool to the db immediately. No waiting
> around for transactions to complete.
>
> Cheers
> Pieter
>
> On 07/10/2015 13:42, Stephen Mallette wrote:
> > Pieter, do you know how Neo4j handles ResourceIterator with respect to
> > closing a Graph.  If I'm iteratng ResourceIterator and another thread
> calls
> > Graph.close() what happens?
> >
> > On Wed, Oct 7, 2015 at 7:35 AM, pieter-gmail <pi...@gmail.com>
> > wrote:
> >
> >> Hi,
> >>
> >> The resource that concerns me most is GraphTraversal as that is almost
> >> always a resource to the underlying db and is created very very often. I
> >> reckon GraphTraversal should also implement AutoCloseable. Further, it
> >> can be auto closed when fully iterated/traversed, for transactional
> >> graphs on commit/rollback or by the user.
> >>
> >> For long running transactions and for GraphTraversals that are not fully
> >> traversed the user can manage the resource by closing it.
> >>
> >> This is similar to how Neo4j does it with its ResourceIterator which is
> >> an AutoCloseable
> >>
> >> Thanks
> >> Pieter
> >>
> >> On 05/10/2015 21:56, Stephen Mallette wrote:
> >>> There are two tickets in JIRA that relate to the semantics of closing
> and
> >>> releasing resources in the Graph hierarchy:
> >>>
> >>> + Graph
> >>> + TraversalSource
> >>> + TraversalStrategy
> >>> + GraphTraversal
> >>> + Transaction
> >>>
> >>> https://issues.apache.org/jira/browse/TINKERPOP3-789
> >>> https://issues.apache.org/jira/browse/TINKERPOP3-790
> >>>
> >>> There's been some discussion on them already.  I bring this to
> everyone's
> >>> attention as the change could have wide repercussions depending on the
> >>> direction it goes.  The short of the matter is that currently:
> >>>
> >>> 1. Graph implements AutoCloseable, but we don't enforce the notion of
> >>> close() itself in the test suite.
> >>> 2. TraversalSource should likely implement AutoCloseable as there are
> >>> sometimes resources that need to be released when a TraversalSource is
> no
> >>> longer in use.
> >>> 3. Transaction implements AutoCloseable but it applies in the context
> of
> >>> how a  Transaction will behave when Graph.close() is called.
> >>>
> >>> Where is this all going?  Matt Frantz summarized the thought points
> >> nicely
> >>> in this comment:
> >>>
> >>>
> >>
> https://issues.apache.org/jira/browse/TINKERPOP3-790?focusedCommentId=14710389&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14710389
> >>> Basically, there's two options Matt identified:
> >>>
> >>> 1. Making it so that Graph.close() would release resources and
> >> effectively
> >>> close() all things it spawned is one approach, but I''m not sure how
> >>> straightforward/practical to implement that is.  I suspect it will
> >>> complicate Graph implementations as well.
> >>> 2. The alternative is that the things in this hierarchy can have their
> >> own
> >>> expensive resources which can be independently closed.  I think that
> this
> >>> approach more closely fits with the code as we have it now and won't
> >> unduly
> >>> burden implementers.  TinkerPop Developers will have to know when to
> call
> >>> close() (e.g. if a TraversalStrategy has expensive resources and the
> user
> >>> assumes that a call to TraversalSource.close() will release those
> >>> resources, then they might be wrong - depends on the approach we decide
> >>> on).
> >>>
> >>> When I first created 790, i had the second option in mind, but since
> Matt
> >>> brought wrote that comment, i figured it was worth thinking through as
> a
> >>> whole.
> >>>
> >>> Once that is decided then we should figure out 789 which is how to
> >> enforce
> >>> the semantics of close() (e.g. what does a Graph do when it's closed
> and
> >>> someone calls graph.traversal()?).
> >>>
> >>> Thoughts?
> >>>
> >>
>
>

Re: [DISCUSS] Semantics of close()

Posted by pieter-gmail <pi...@gmail.com>.
No I don't know. Their GraphDatabaseService.shutdown() javadoc says,

"Shuts down Neo4j. After this method has been invoked, it's invalid to
invoke any methods in the Neo4j API and all references to this instance
of GraphDatabaseService should be discarded."

That sorta means that they will clean up all dangling resources implicitly.

In sqlg I close the connection pool to the db immediately. No waiting
around for transactions to complete.

Cheers
Pieter

On 07/10/2015 13:42, Stephen Mallette wrote:
> Pieter, do you know how Neo4j handles ResourceIterator with respect to
> closing a Graph.  If I'm iteratng ResourceIterator and another thread calls
> Graph.close() what happens?
>
> On Wed, Oct 7, 2015 at 7:35 AM, pieter-gmail <pi...@gmail.com>
> wrote:
>
>> Hi,
>>
>> The resource that concerns me most is GraphTraversal as that is almost
>> always a resource to the underlying db and is created very very often. I
>> reckon GraphTraversal should also implement AutoCloseable. Further, it
>> can be auto closed when fully iterated/traversed, for transactional
>> graphs on commit/rollback or by the user.
>>
>> For long running transactions and for GraphTraversals that are not fully
>> traversed the user can manage the resource by closing it.
>>
>> This is similar to how Neo4j does it with its ResourceIterator which is
>> an AutoCloseable
>>
>> Thanks
>> Pieter
>>
>> On 05/10/2015 21:56, Stephen Mallette wrote:
>>> There are two tickets in JIRA that relate to the semantics of closing and
>>> releasing resources in the Graph hierarchy:
>>>
>>> + Graph
>>> + TraversalSource
>>> + TraversalStrategy
>>> + GraphTraversal
>>> + Transaction
>>>
>>> https://issues.apache.org/jira/browse/TINKERPOP3-789
>>> https://issues.apache.org/jira/browse/TINKERPOP3-790
>>>
>>> There's been some discussion on them already.  I bring this to everyone's
>>> attention as the change could have wide repercussions depending on the
>>> direction it goes.  The short of the matter is that currently:
>>>
>>> 1. Graph implements AutoCloseable, but we don't enforce the notion of
>>> close() itself in the test suite.
>>> 2. TraversalSource should likely implement AutoCloseable as there are
>>> sometimes resources that need to be released when a TraversalSource is no
>>> longer in use.
>>> 3. Transaction implements AutoCloseable but it applies in the context of
>>> how a  Transaction will behave when Graph.close() is called.
>>>
>>> Where is this all going?  Matt Frantz summarized the thought points
>> nicely
>>> in this comment:
>>>
>>>
>> https://issues.apache.org/jira/browse/TINKERPOP3-790?focusedCommentId=14710389&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14710389
>>> Basically, there's two options Matt identified:
>>>
>>> 1. Making it so that Graph.close() would release resources and
>> effectively
>>> close() all things it spawned is one approach, but I''m not sure how
>>> straightforward/practical to implement that is.  I suspect it will
>>> complicate Graph implementations as well.
>>> 2. The alternative is that the things in this hierarchy can have their
>> own
>>> expensive resources which can be independently closed.  I think that this
>>> approach more closely fits with the code as we have it now and won't
>> unduly
>>> burden implementers.  TinkerPop Developers will have to know when to call
>>> close() (e.g. if a TraversalStrategy has expensive resources and the user
>>> assumes that a call to TraversalSource.close() will release those
>>> resources, then they might be wrong - depends on the approach we decide
>>> on).
>>>
>>> When I first created 790, i had the second option in mind, but since Matt
>>> brought wrote that comment, i figured it was worth thinking through as a
>>> whole.
>>>
>>> Once that is decided then we should figure out 789 which is how to
>> enforce
>>> the semantics of close() (e.g. what does a Graph do when it's closed and
>>> someone calls graph.traversal()?).
>>>
>>> Thoughts?
>>>
>>


Re: [DISCUSS] Semantics of close()

Posted by Stephen Mallette <sp...@gmail.com>.
Pieter, do you know how Neo4j handles ResourceIterator with respect to
closing a Graph.  If I'm iteratng ResourceIterator and another thread calls
Graph.close() what happens?

On Wed, Oct 7, 2015 at 7:35 AM, pieter-gmail <pi...@gmail.com>
wrote:

> Hi,
>
> The resource that concerns me most is GraphTraversal as that is almost
> always a resource to the underlying db and is created very very often. I
> reckon GraphTraversal should also implement AutoCloseable. Further, it
> can be auto closed when fully iterated/traversed, for transactional
> graphs on commit/rollback or by the user.
>
> For long running transactions and for GraphTraversals that are not fully
> traversed the user can manage the resource by closing it.
>
> This is similar to how Neo4j does it with its ResourceIterator which is
> an AutoCloseable
>
> Thanks
> Pieter
>
> On 05/10/2015 21:56, Stephen Mallette wrote:
> > There are two tickets in JIRA that relate to the semantics of closing and
> > releasing resources in the Graph hierarchy:
> >
> > + Graph
> > + TraversalSource
> > + TraversalStrategy
> > + GraphTraversal
> > + Transaction
> >
> > https://issues.apache.org/jira/browse/TINKERPOP3-789
> > https://issues.apache.org/jira/browse/TINKERPOP3-790
> >
> > There's been some discussion on them already.  I bring this to everyone's
> > attention as the change could have wide repercussions depending on the
> > direction it goes.  The short of the matter is that currently:
> >
> > 1. Graph implements AutoCloseable, but we don't enforce the notion of
> > close() itself in the test suite.
> > 2. TraversalSource should likely implement AutoCloseable as there are
> > sometimes resources that need to be released when a TraversalSource is no
> > longer in use.
> > 3. Transaction implements AutoCloseable but it applies in the context of
> > how a  Transaction will behave when Graph.close() is called.
> >
> > Where is this all going?  Matt Frantz summarized the thought points
> nicely
> > in this comment:
> >
> >
> https://issues.apache.org/jira/browse/TINKERPOP3-790?focusedCommentId=14710389&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14710389
> >
> > Basically, there's two options Matt identified:
> >
> > 1. Making it so that Graph.close() would release resources and
> effectively
> > close() all things it spawned is one approach, but I''m not sure how
> > straightforward/practical to implement that is.  I suspect it will
> > complicate Graph implementations as well.
> > 2. The alternative is that the things in this hierarchy can have their
> own
> > expensive resources which can be independently closed.  I think that this
> > approach more closely fits with the code as we have it now and won't
> unduly
> > burden implementers.  TinkerPop Developers will have to know when to call
> > close() (e.g. if a TraversalStrategy has expensive resources and the user
> > assumes that a call to TraversalSource.close() will release those
> > resources, then they might be wrong - depends on the approach we decide
> > on).
> >
> > When I first created 790, i had the second option in mind, but since Matt
> > brought wrote that comment, i figured it was worth thinking through as a
> > whole.
> >
> > Once that is decided then we should figure out 789 which is how to
> enforce
> > the semantics of close() (e.g. what does a Graph do when it's closed and
> > someone calls graph.traversal()?).
> >
> > Thoughts?
> >
>
>

Re: [DISCUSS] Semantics of close()

Posted by pieter-gmail <pi...@gmail.com>.
Hi,

The resource that concerns me most is GraphTraversal as that is almost
always a resource to the underlying db and is created very very often. I
reckon GraphTraversal should also implement AutoCloseable. Further, it
can be auto closed when fully iterated/traversed, for transactional
graphs on commit/rollback or by the user.

For long running transactions and for GraphTraversals that are not fully
traversed the user can manage the resource by closing it.

This is similar to how Neo4j does it with its ResourceIterator which is
an AutoCloseable

Thanks
Pieter

On 05/10/2015 21:56, Stephen Mallette wrote:
> There are two tickets in JIRA that relate to the semantics of closing and
> releasing resources in the Graph hierarchy:
>
> + Graph
> + TraversalSource
> + TraversalStrategy
> + GraphTraversal
> + Transaction
>
> https://issues.apache.org/jira/browse/TINKERPOP3-789
> https://issues.apache.org/jira/browse/TINKERPOP3-790
>
> There's been some discussion on them already.  I bring this to everyone's
> attention as the change could have wide repercussions depending on the
> direction it goes.  The short of the matter is that currently:
>
> 1. Graph implements AutoCloseable, but we don't enforce the notion of
> close() itself in the test suite.
> 2. TraversalSource should likely implement AutoCloseable as there are
> sometimes resources that need to be released when a TraversalSource is no
> longer in use.
> 3. Transaction implements AutoCloseable but it applies in the context of
> how a  Transaction will behave when Graph.close() is called.
>
> Where is this all going?  Matt Frantz summarized the thought points nicely
> in this comment:
>
> https://issues.apache.org/jira/browse/TINKERPOP3-790?focusedCommentId=14710389&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14710389
>
> Basically, there's two options Matt identified:
>
> 1. Making it so that Graph.close() would release resources and effectively
> close() all things it spawned is one approach, but I''m not sure how
> straightforward/practical to implement that is.  I suspect it will
> complicate Graph implementations as well.
> 2. The alternative is that the things in this hierarchy can have their own
> expensive resources which can be independently closed.  I think that this
> approach more closely fits with the code as we have it now and won't unduly
> burden implementers.  TinkerPop Developers will have to know when to call
> close() (e.g. if a TraversalStrategy has expensive resources and the user
> assumes that a call to TraversalSource.close() will release those
> resources, then they might be wrong - depends on the approach we decide
> on).
>
> When I first created 790, i had the second option in mind, but since Matt
> brought wrote that comment, i figured it was worth thinking through as a
> whole.
>
> Once that is decided then we should figure out 789 which is how to enforce
> the semantics of close() (e.g. what does a Graph do when it's closed and
> someone calls graph.traversal()?).
>
> Thoughts?
>


Re: [DISCUSS] Semantics of close()

Posted by Bryn Cooke <br...@gmail.com>.
I had misunderstood. If close already blocks then I'm not in favour of 
adding an async version. Users can roll get a future by executing close 
via a thread pool if required.



On 06/10/15 15:10, Stephen Mallette wrote:
> i probably should have added in my last post that AutoCloseable.close()
> would already be our blocking method, no?.  You're trying to add an async
> type method to Graph, no?
>
> Future closeAsync()
>
> On Tue, Oct 6, 2015 at 10:05 AM, Bryn Cooke <br...@gmail.com> wrote:
>
>>
>>> a Future makes sense though that would be a separate method right?
>>> AutoCloseable.close() doesn't return Future.
>>>
>> Perhaps the new method could be called 'join' and it blocks until the
>> graph is fully closed.
>> I'm taking inspiration from embedded jetty.
>>
>>
>>
>>
>> On Tue, Oct 6, 2015 at 9:15 AM, Bryn Cooke <br...@gmail.com> wrote:
>>> Hi,
>>>> I'm not sure what the answer is here,
>>>>
>>>> What I am sure of is that the goal of close should be a graceful
>>>> shutdown.
>>>> Once a resource is closed then it should not accept new requests.
>>>> Descendant resources may or may not continue to be operational, for
>>>> instance you may allow ongoing transactions to complete but that is
>>>> implementation dependent.
>>>>
>>>> The question is after calling close when how can you be sure that all
>>>> descendant resources are finished before returning to the user? Only by
>>>> keeping references to all spawned resources can you tell.
>>>>
>>>> Perhaps Graph.close() should return a Future. That way you can block
>>>> until
>>>> the graph is actually closed.
>>>>
>>>> Bryn
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 05/10/15 20:56, Stephen Mallette wrote:
>>>>
>>>> There are two tickets in JIRA that relate to the semantics of closing and
>>>>> releasing resources in the Graph hierarchy:
>>>>>
>>>>> + Graph
>>>>> + TraversalSource
>>>>> + TraversalStrategy
>>>>> + GraphTraversal
>>>>> + Transaction
>>>>>
>>>>> https://issues.apache.org/jira/browse/TINKERPOP3-789
>>>>> https://issues.apache.org/jira/browse/TINKERPOP3-790
>>>>>
>>>>> There's been some discussion on them already.  I bring this to
>>>>> everyone's
>>>>> attention as the change could have wide repercussions depending on the
>>>>> direction it goes.  The short of the matter is that currently:
>>>>>
>>>>> 1. Graph implements AutoCloseable, but we don't enforce the notion of
>>>>> close() itself in the test suite.
>>>>> 2. TraversalSource should likely implement AutoCloseable as there are
>>>>> sometimes resources that need to be released when a TraversalSource is
>>>>> no
>>>>> longer in use.
>>>>> 3. Transaction implements AutoCloseable but it applies in the context of
>>>>> how a  Transaction will behave when Graph.close() is called.
>>>>>
>>>>> Where is this all going?  Matt Frantz summarized the thought points
>>>>> nicely
>>>>> in this comment:
>>>>>
>>>>>
>>>>>
>>>>> https://issues.apache.org/jira/browse/TINKERPOP3-790?focusedCommentId=14710389&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14710389
>>>>>
>>>>> Basically, there's two options Matt identified:
>>>>>
>>>>> 1. Making it so that Graph.close() would release resources and
>>>>> effectively
>>>>> close() all things it spawned is one approach, but I''m not sure how
>>>>> straightforward/practical to implement that is.  I suspect it will
>>>>> complicate Graph implementations as well.
>>>>> 2. The alternative is that the things in this hierarchy can have their
>>>>> own
>>>>> expensive resources which can be independently closed.  I think that
>>>>> this
>>>>> approach more closely fits with the code as we have it now and won't
>>>>> unduly
>>>>> burden implementers.  TinkerPop Developers will have to know when to
>>>>> call
>>>>> close() (e.g. if a TraversalStrategy has expensive resources and the
>>>>> user
>>>>> assumes that a call to TraversalSource.close() will release those
>>>>> resources, then they might be wrong - depends on the approach we decide
>>>>> on).
>>>>>
>>>>> When I first created 790, i had the second option in mind, but since
>>>>> Matt
>>>>> brought wrote that comment, i figured it was worth thinking through as a
>>>>> whole.
>>>>>
>>>>> Once that is decided then we should figure out 789 which is how to
>>>>> enforce
>>>>> the semantics of close() (e.g. what does a Graph do when it's closed and
>>>>> someone calls graph.traversal()?).
>>>>>
>>>>> Thoughts?
>>>>>
>>>>>
>>>>>


Re: [DISCUSS] Semantics of close()

Posted by Stephen Mallette <sp...@gmail.com>.
i probably should have added in my last post that AutoCloseable.close()
would already be our blocking method, no?.  You're trying to add an async
type method to Graph, no?

Future closeAsync()

On Tue, Oct 6, 2015 at 10:05 AM, Bryn Cooke <br...@gmail.com> wrote:

>
>
>> a Future makes sense though that would be a separate method right?
>> AutoCloseable.close() doesn't return Future.
>>
>
> Perhaps the new method could be called 'join' and it blocks until the
> graph is fully closed.
> I'm taking inspiration from embedded jetty.
>
>
>
>
> On Tue, Oct 6, 2015 at 9:15 AM, Bryn Cooke <br...@gmail.com> wrote:
>>
>> Hi,
>>>
>>> I'm not sure what the answer is here,
>>>
>>> What I am sure of is that the goal of close should be a graceful
>>> shutdown.
>>> Once a resource is closed then it should not accept new requests.
>>> Descendant resources may or may not continue to be operational, for
>>> instance you may allow ongoing transactions to complete but that is
>>> implementation dependent.
>>>
>>> The question is after calling close when how can you be sure that all
>>> descendant resources are finished before returning to the user? Only by
>>> keeping references to all spawned resources can you tell.
>>>
>>> Perhaps Graph.close() should return a Future. That way you can block
>>> until
>>> the graph is actually closed.
>>>
>>> Bryn
>>>
>>>
>>>
>>>
>>>
>>> On 05/10/15 20:56, Stephen Mallette wrote:
>>>
>>> There are two tickets in JIRA that relate to the semantics of closing and
>>>> releasing resources in the Graph hierarchy:
>>>>
>>>> + Graph
>>>> + TraversalSource
>>>> + TraversalStrategy
>>>> + GraphTraversal
>>>> + Transaction
>>>>
>>>> https://issues.apache.org/jira/browse/TINKERPOP3-789
>>>> https://issues.apache.org/jira/browse/TINKERPOP3-790
>>>>
>>>> There's been some discussion on them already.  I bring this to
>>>> everyone's
>>>> attention as the change could have wide repercussions depending on the
>>>> direction it goes.  The short of the matter is that currently:
>>>>
>>>> 1. Graph implements AutoCloseable, but we don't enforce the notion of
>>>> close() itself in the test suite.
>>>> 2. TraversalSource should likely implement AutoCloseable as there are
>>>> sometimes resources that need to be released when a TraversalSource is
>>>> no
>>>> longer in use.
>>>> 3. Transaction implements AutoCloseable but it applies in the context of
>>>> how a  Transaction will behave when Graph.close() is called.
>>>>
>>>> Where is this all going?  Matt Frantz summarized the thought points
>>>> nicely
>>>> in this comment:
>>>>
>>>>
>>>>
>>>> https://issues.apache.org/jira/browse/TINKERPOP3-790?focusedCommentId=14710389&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14710389
>>>>
>>>> Basically, there's two options Matt identified:
>>>>
>>>> 1. Making it so that Graph.close() would release resources and
>>>> effectively
>>>> close() all things it spawned is one approach, but I''m not sure how
>>>> straightforward/practical to implement that is.  I suspect it will
>>>> complicate Graph implementations as well.
>>>> 2. The alternative is that the things in this hierarchy can have their
>>>> own
>>>> expensive resources which can be independently closed.  I think that
>>>> this
>>>> approach more closely fits with the code as we have it now and won't
>>>> unduly
>>>> burden implementers.  TinkerPop Developers will have to know when to
>>>> call
>>>> close() (e.g. if a TraversalStrategy has expensive resources and the
>>>> user
>>>> assumes that a call to TraversalSource.close() will release those
>>>> resources, then they might be wrong - depends on the approach we decide
>>>> on).
>>>>
>>>> When I first created 790, i had the second option in mind, but since
>>>> Matt
>>>> brought wrote that comment, i figured it was worth thinking through as a
>>>> whole.
>>>>
>>>> Once that is decided then we should figure out 789 which is how to
>>>> enforce
>>>> the semantics of close() (e.g. what does a Graph do when it's closed and
>>>> someone calls graph.traversal()?).
>>>>
>>>> Thoughts?
>>>>
>>>>
>>>>
>

Re: [DISCUSS] Semantics of close()

Posted by Bryn Cooke <br...@gmail.com>.
>
> a Future makes sense though that would be a separate method right?
> AutoCloseable.close() doesn't return Future.

Perhaps the new method could be called 'join' and it blocks until the 
graph is fully closed.
I'm taking inspiration from embedded jetty.



> On Tue, Oct 6, 2015 at 9:15 AM, Bryn Cooke <br...@gmail.com> wrote:
>
>> Hi,
>>
>> I'm not sure what the answer is here,
>>
>> What I am sure of is that the goal of close should be a graceful shutdown.
>> Once a resource is closed then it should not accept new requests.
>> Descendant resources may or may not continue to be operational, for
>> instance you may allow ongoing transactions to complete but that is
>> implementation dependent.
>>
>> The question is after calling close when how can you be sure that all
>> descendant resources are finished before returning to the user? Only by
>> keeping references to all spawned resources can you tell.
>>
>> Perhaps Graph.close() should return a Future. That way you can block until
>> the graph is actually closed.
>>
>> Bryn
>>
>>
>>
>>
>>
>> On 05/10/15 20:56, Stephen Mallette wrote:
>>
>>> There are two tickets in JIRA that relate to the semantics of closing and
>>> releasing resources in the Graph hierarchy:
>>>
>>> + Graph
>>> + TraversalSource
>>> + TraversalStrategy
>>> + GraphTraversal
>>> + Transaction
>>>
>>> https://issues.apache.org/jira/browse/TINKERPOP3-789
>>> https://issues.apache.org/jira/browse/TINKERPOP3-790
>>>
>>> There's been some discussion on them already.  I bring this to everyone's
>>> attention as the change could have wide repercussions depending on the
>>> direction it goes.  The short of the matter is that currently:
>>>
>>> 1. Graph implements AutoCloseable, but we don't enforce the notion of
>>> close() itself in the test suite.
>>> 2. TraversalSource should likely implement AutoCloseable as there are
>>> sometimes resources that need to be released when a TraversalSource is no
>>> longer in use.
>>> 3. Transaction implements AutoCloseable but it applies in the context of
>>> how a  Transaction will behave when Graph.close() is called.
>>>
>>> Where is this all going?  Matt Frantz summarized the thought points nicely
>>> in this comment:
>>>
>>>
>>> https://issues.apache.org/jira/browse/TINKERPOP3-790?focusedCommentId=14710389&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14710389
>>>
>>> Basically, there's two options Matt identified:
>>>
>>> 1. Making it so that Graph.close() would release resources and effectively
>>> close() all things it spawned is one approach, but I''m not sure how
>>> straightforward/practical to implement that is.  I suspect it will
>>> complicate Graph implementations as well.
>>> 2. The alternative is that the things in this hierarchy can have their own
>>> expensive resources which can be independently closed.  I think that this
>>> approach more closely fits with the code as we have it now and won't
>>> unduly
>>> burden implementers.  TinkerPop Developers will have to know when to call
>>> close() (e.g. if a TraversalStrategy has expensive resources and the user
>>> assumes that a call to TraversalSource.close() will release those
>>> resources, then they might be wrong - depends on the approach we decide
>>> on).
>>>
>>> When I first created 790, i had the second option in mind, but since Matt
>>> brought wrote that comment, i figured it was worth thinking through as a
>>> whole.
>>>
>>> Once that is decided then we should figure out 789 which is how to enforce
>>> the semantics of close() (e.g. what does a Graph do when it's closed and
>>> someone calls graph.traversal()?).
>>>
>>> Thoughts?
>>>
>>>


Re: [DISCUSS] Semantics of close()

Posted by Stephen Mallette <sp...@gmail.com>.
Thanks for the thoughts, Bryn.   sounds like you are of the opinion that we
should maintain a strong hierarchy of close() not allowing other resources
to live on.

The question is after calling close when how can you be sure that all
> descendant resources are finished before returning to the user? Only by
> keeping references to all spawned resources can you tell.
>

yeah - that's what complicates the implementations and i'm sorta not in
favor of that, but that would be a proper implementation.


> Perhaps Graph.close() should return a Future. That way you can block until
> the graph is actually closed.


a Future makes sense though that would be a separate method right?
AutoCloseable.close() doesn't return Future.

On Tue, Oct 6, 2015 at 9:15 AM, Bryn Cooke <br...@gmail.com> wrote:

> Hi,
>
> I'm not sure what the answer is here,
>
> What I am sure of is that the goal of close should be a graceful shutdown.
> Once a resource is closed then it should not accept new requests.
> Descendant resources may or may not continue to be operational, for
> instance you may allow ongoing transactions to complete but that is
> implementation dependent.
>
> The question is after calling close when how can you be sure that all
> descendant resources are finished before returning to the user? Only by
> keeping references to all spawned resources can you tell.
>
> Perhaps Graph.close() should return a Future. That way you can block until
> the graph is actually closed.
>
> Bryn
>
>
>
>
>
> On 05/10/15 20:56, Stephen Mallette wrote:
>
>> There are two tickets in JIRA that relate to the semantics of closing and
>> releasing resources in the Graph hierarchy:
>>
>> + Graph
>> + TraversalSource
>> + TraversalStrategy
>> + GraphTraversal
>> + Transaction
>>
>> https://issues.apache.org/jira/browse/TINKERPOP3-789
>> https://issues.apache.org/jira/browse/TINKERPOP3-790
>>
>> There's been some discussion on them already.  I bring this to everyone's
>> attention as the change could have wide repercussions depending on the
>> direction it goes.  The short of the matter is that currently:
>>
>> 1. Graph implements AutoCloseable, but we don't enforce the notion of
>> close() itself in the test suite.
>> 2. TraversalSource should likely implement AutoCloseable as there are
>> sometimes resources that need to be released when a TraversalSource is no
>> longer in use.
>> 3. Transaction implements AutoCloseable but it applies in the context of
>> how a  Transaction will behave when Graph.close() is called.
>>
>> Where is this all going?  Matt Frantz summarized the thought points nicely
>> in this comment:
>>
>>
>> https://issues.apache.org/jira/browse/TINKERPOP3-790?focusedCommentId=14710389&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14710389
>>
>> Basically, there's two options Matt identified:
>>
>> 1. Making it so that Graph.close() would release resources and effectively
>> close() all things it spawned is one approach, but I''m not sure how
>> straightforward/practical to implement that is.  I suspect it will
>> complicate Graph implementations as well.
>> 2. The alternative is that the things in this hierarchy can have their own
>> expensive resources which can be independently closed.  I think that this
>> approach more closely fits with the code as we have it now and won't
>> unduly
>> burden implementers.  TinkerPop Developers will have to know when to call
>> close() (e.g. if a TraversalStrategy has expensive resources and the user
>> assumes that a call to TraversalSource.close() will release those
>> resources, then they might be wrong - depends on the approach we decide
>> on).
>>
>> When I first created 790, i had the second option in mind, but since Matt
>> brought wrote that comment, i figured it was worth thinking through as a
>> whole.
>>
>> Once that is decided then we should figure out 789 which is how to enforce
>> the semantics of close() (e.g. what does a Graph do when it's closed and
>> someone calls graph.traversal()?).
>>
>> Thoughts?
>>
>>
>

Re: [DISCUSS] Semantics of close()

Posted by Bryn Cooke <br...@gmail.com>.
Hi,

I'm not sure what the answer is here,

What I am sure of is that the goal of close should be a graceful 
shutdown. Once a resource is closed then it should not accept new 
requests. Descendant resources may or may not continue to be 
operational, for instance you may allow ongoing transactions to complete 
but that is implementation dependent.

The question is after calling close when how can you be sure that all 
descendant resources are finished before returning to the user? Only by 
keeping references to all spawned resources can you tell.

Perhaps Graph.close() should return a Future. That way you can block 
until the graph is actually closed.

Bryn




On 05/10/15 20:56, Stephen Mallette wrote:
> There are two tickets in JIRA that relate to the semantics of closing and
> releasing resources in the Graph hierarchy:
>
> + Graph
> + TraversalSource
> + TraversalStrategy
> + GraphTraversal
> + Transaction
>
> https://issues.apache.org/jira/browse/TINKERPOP3-789
> https://issues.apache.org/jira/browse/TINKERPOP3-790
>
> There's been some discussion on them already.  I bring this to everyone's
> attention as the change could have wide repercussions depending on the
> direction it goes.  The short of the matter is that currently:
>
> 1. Graph implements AutoCloseable, but we don't enforce the notion of
> close() itself in the test suite.
> 2. TraversalSource should likely implement AutoCloseable as there are
> sometimes resources that need to be released when a TraversalSource is no
> longer in use.
> 3. Transaction implements AutoCloseable but it applies in the context of
> how a  Transaction will behave when Graph.close() is called.
>
> Where is this all going?  Matt Frantz summarized the thought points nicely
> in this comment:
>
> https://issues.apache.org/jira/browse/TINKERPOP3-790?focusedCommentId=14710389&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14710389
>
> Basically, there's two options Matt identified:
>
> 1. Making it so that Graph.close() would release resources and effectively
> close() all things it spawned is one approach, but I''m not sure how
> straightforward/practical to implement that is.  I suspect it will
> complicate Graph implementations as well.
> 2. The alternative is that the things in this hierarchy can have their own
> expensive resources which can be independently closed.  I think that this
> approach more closely fits with the code as we have it now and won't unduly
> burden implementers.  TinkerPop Developers will have to know when to call
> close() (e.g. if a TraversalStrategy has expensive resources and the user
> assumes that a call to TraversalSource.close() will release those
> resources, then they might be wrong - depends on the approach we decide
> on).
>
> When I first created 790, i had the second option in mind, but since Matt
> brought wrote that comment, i figured it was worth thinking through as a
> whole.
>
> Once that is decided then we should figure out 789 which is how to enforce
> the semantics of close() (e.g. what does a Graph do when it's closed and
> someone calls graph.traversal()?).
>
> Thoughts?
>