Re: [DSISCUSS] Remote Transactions

2021-03-17 Thread Stephen Mallette
One thing I should have noted about tx().create(). The create() very well
could have been named traversal(), thus the previous example reading as:

g = traversal().withEmbedded(graph)
// or
g = traversal().withRemote(conn)
gtx = g.tx().traversal()
gtx.addV('person').iterate()
gtx.addV('software').iterate()
gtx.close() // alternatively you could explicitly commit() or rollback()

You're basically spawning a new GraphTraversalSource from the Transaction
object rather than from a Graph or AnonymousTraversal. I chose create()
because it felt like this looked weird:

g = traversal().withRemote(conn).tx().traversal()

which would be a weird thing to do I guess, but just seeing "traversal()"
over and over seemed odd looking and I wanted to differentiate with the
Transaction object even though the methods are identical in what they do. I
suppose I also drew inspiration from:

Transaction.createdThreadedTx()

which I think we might consider deprecating. JanusGraph would simply expose
their own Transaction object in the future that has that method as I
imagine folks still use that feature. As far as I know, no other graph
implements that functionality and my guess is that no one will likely do so
in the future.

anyway, if you like traversal() better than create() or the other way
around, please let me know

On Wed, Mar 17, 2021 at 5:33 PM David Bechberger 
wrote:

> I am in favor of this change as I any idea that unifies the multiple
> different ways things work in TP will only make it easier to learn and
> adopt.
>
> I don't know that I have enough knowledge of the inner workings of
> transactions to know what/if this will cause problems.
>
> Dave
>
> On Wed, Mar 17, 2021 at 12:57 PM Stephen Mallette 
> wrote:
>
> > We haven't touched "transactions" since TP3 was originally designed. They
> > have remained a feature for embedded use cases even in the face of the
> rise
> > of remote graph use cases and result in major inconsistencies that really
> > bother users.
> >
> > As we close on 3.5.0, I figured that perhaps there was a chance to do
> > something radical about transactions. Basically, I'd like transactions to
> > work irrespective of remote or embedded usage and for them to have the
> same
> > API when doing so. In mulling it over for the last day or so I had a
> > realization that makes me believe that the following is possible:
> >
> > g = traversal().withEmbedded(graph)
> > // or
> > g = traversal().withRemote(conn)
> > gtx = g.tx().create()
> > gtx.addV('person').iterate()
> > gtx.addV('software').iterate()
> > gtx.close() // alternatively you could explicitly commit() or rollback()
> >
> > // you could still use g for sessionless, but gtx is "done" after
> > // you close so you will need to create() a new gtx instance for a
> > // fresh transaction
> > assert 2 == g.V().count().next()
> >
> > Note that the create() method on tx() is the new bit of necessary syntax.
> > Technically, it is a do-nothing for embedded mode (and you could skip it
> > for thread-local auto-transactions) but all the documentation can be
> > shifted so that the API is identical to remote. The change would be
> > non-breaking as the embedded transaction approach would remain as it is,
> > but would no longer be documented as the preferred approach. Perhaps we
> > could one day disallow it but it's a bit of a tangle of ThreadLocal that
> > I'm not sure we want to touch in TP3.
> >
> > What happens behind the scenes of g.tx().create() is that in remote cases
> > the RemoteConnection constructs a remote Transaction implementation which
> > basically is used to spawn new traversal source instances with a session
> > based connections. The remote Transaction object can't support
> transaction
> > listeners or special read-write/close configurations, but I don't think
> > that's a problem as those things don't make a lot of sense in remote use
> > cases.
> >
> > From the server perspective, this change would also mean that sessions
> > would have to accept bytecode and not just scripts, which technically
> > shouldn't be a problem.
> >
> > One downside at the moment is that i'm thinking mostly in Java at the
> > moment. I'm not sure how this all works in other variants just yet, but
> I'd
> > hope to keep similar syntax.
> >
> > I will keep experimenting tomorrow. If you have any thoughts on the
> matter,
> > I'd be happy to hear them. Thanks!
> >
>


Re: [DSISCUSS] Remote Transactions

2021-03-17 Thread David Bechberger
I am in favor of this change as I any idea that unifies the multiple
different ways things work in TP will only make it easier to learn and
adopt.

I don't know that I have enough knowledge of the inner workings of
transactions to know what/if this will cause problems.

Dave

On Wed, Mar 17, 2021 at 12:57 PM Stephen Mallette 
wrote:

> We haven't touched "transactions" since TP3 was originally designed. They
> have remained a feature for embedded use cases even in the face of the rise
> of remote graph use cases and result in major inconsistencies that really
> bother users.
>
> As we close on 3.5.0, I figured that perhaps there was a chance to do
> something radical about transactions. Basically, I'd like transactions to
> work irrespective of remote or embedded usage and for them to have the same
> API when doing so. In mulling it over for the last day or so I had a
> realization that makes me believe that the following is possible:
>
> g = traversal().withEmbedded(graph)
> // or
> g = traversal().withRemote(conn)
> gtx = g.tx().create()
> gtx.addV('person').iterate()
> gtx.addV('software').iterate()
> gtx.close() // alternatively you could explicitly commit() or rollback()
>
> // you could still use g for sessionless, but gtx is "done" after
> // you close so you will need to create() a new gtx instance for a
> // fresh transaction
> assert 2 == g.V().count().next()
>
> Note that the create() method on tx() is the new bit of necessary syntax.
> Technically, it is a do-nothing for embedded mode (and you could skip it
> for thread-local auto-transactions) but all the documentation can be
> shifted so that the API is identical to remote. The change would be
> non-breaking as the embedded transaction approach would remain as it is,
> but would no longer be documented as the preferred approach. Perhaps we
> could one day disallow it but it's a bit of a tangle of ThreadLocal that
> I'm not sure we want to touch in TP3.
>
> What happens behind the scenes of g.tx().create() is that in remote cases
> the RemoteConnection constructs a remote Transaction implementation which
> basically is used to spawn new traversal source instances with a session
> based connections. The remote Transaction object can't support transaction
> listeners or special read-write/close configurations, but I don't think
> that's a problem as those things don't make a lot of sense in remote use
> cases.
>
> From the server perspective, this change would also mean that sessions
> would have to accept bytecode and not just scripts, which technically
> shouldn't be a problem.
>
> One downside at the moment is that i'm thinking mostly in Java at the
> moment. I'm not sure how this all works in other variants just yet, but I'd
> hope to keep similar syntax.
>
> I will keep experimenting tomorrow. If you have any thoughts on the matter,
> I'd be happy to hear them. Thanks!
>


[DSISCUSS] Remote Transactions

2021-03-17 Thread Stephen Mallette
We haven't touched "transactions" since TP3 was originally designed. They
have remained a feature for embedded use cases even in the face of the rise
of remote graph use cases and result in major inconsistencies that really
bother users.

As we close on 3.5.0, I figured that perhaps there was a chance to do
something radical about transactions. Basically, I'd like transactions to
work irrespective of remote or embedded usage and for them to have the same
API when doing so. In mulling it over for the last day or so I had a
realization that makes me believe that the following is possible:

g = traversal().withEmbedded(graph)
// or
g = traversal().withRemote(conn)
gtx = g.tx().create()
gtx.addV('person').iterate()
gtx.addV('software').iterate()
gtx.close() // alternatively you could explicitly commit() or rollback()

// you could still use g for sessionless, but gtx is "done" after
// you close so you will need to create() a new gtx instance for a
// fresh transaction
assert 2 == g.V().count().next()

Note that the create() method on tx() is the new bit of necessary syntax.
Technically, it is a do-nothing for embedded mode (and you could skip it
for thread-local auto-transactions) but all the documentation can be
shifted so that the API is identical to remote. The change would be
non-breaking as the embedded transaction approach would remain as it is,
but would no longer be documented as the preferred approach. Perhaps we
could one day disallow it but it's a bit of a tangle of ThreadLocal that
I'm not sure we want to touch in TP3.

What happens behind the scenes of g.tx().create() is that in remote cases
the RemoteConnection constructs a remote Transaction implementation which
basically is used to spawn new traversal source instances with a session
based connections. The remote Transaction object can't support transaction
listeners or special read-write/close configurations, but I don't think
that's a problem as those things don't make a lot of sense in remote use
cases.

>From the server perspective, this change would also mean that sessions
would have to accept bytecode and not just scripts, which technically
shouldn't be a problem.

One downside at the moment is that i'm thinking mostly in Java at the
moment. I'm not sure how this all works in other variants just yet, but I'd
hope to keep similar syntax.

I will keep experimenting tomorrow. If you have any thoughts on the matter,
I'd be happy to hear them. Thanks!


[jira] [Commented] (TINKERPOP-2533) Develop a grammar for Gremlin

2021-03-17 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/TINKERPOP-2533?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17303541#comment-17303541
 ] 

ASF GitHub Bot commented on TINKERPOP-2533:
---

FlorianHockmann commented on a change in pull request #1408:
URL: https://github.com/apache/tinkerpop/pull/1408#discussion_r596168076



##
File path: docs/src/recipes/recommendation.asciidoc
##
@@ -33,7 +33,7 @@ g.addV("person").property("name","alice").
   addV("person").property("name","bob").
   addV("person").property("name","jon").
   addV("person").property("name","jack").
-  addV("person").property("name","jill")iterate()
+  addV("person").property("name","jill").iterate()

Review comment:
   Nice to see that this already uncovered some bugs in our docs :)





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Develop a grammar for Gremlin
> -
>
> Key: TINKERPOP-2533
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2533
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: process
>Affects Versions: 3.5.0
>Reporter: Stephen Mallette
>Assignee: Stephen Mallette
>Priority: Major
>
> An ANTLR grammar for Gremlin would allow us to look at Gremlin as more of a 
> language and to independently consider how that language gets implemented in 
> each variant. By driving the language development through the grammar rather 
> than through Java (as we do now), we can avoid problems where Java-oriented 
> idioms leak into other programming language implementations. 
> For this initial body of work, the contribution will contain:
> * a new {{gremlin-grammar}} maven module
> * which will have a ANTLR4 grammar file 
> * tests for the parser which will extract a corpus of Gremlin traversal from 
> the gherkin tests and the documentation
> This initial contribution will have some limitations initially where certain 
> types of statements from the testing corpus are simply not supported by the 
> grammar as it is today. Much of this will have limitation will have to do 
> with examples using Groovy syntax and lambdas, but there are yet places where 
> the grammar needs further development. Those limitations can be satisfied by 
> other pull requests. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (TINKERPOP-2531) Gremlin .NET driver ConnectionPool can remain without connections if server is down for 1-2 minutes

2021-03-17 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/TINKERPOP-2531?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17303361#comment-17303361
 ] 

ASF GitHub Bot commented on TINKERPOP-2531:
---

FlorianHockmann commented on pull request #1404:
URL: https://github.com/apache/tinkerpop/pull/1404#issuecomment-801046798


   Sure, I can add a changelog entry for this when I merge it.
   
   > Is there an estimated timeline for the next minor release into which this 
change might make it ? That'll be 3.4.11 right ?
   
   3.4.11 will be the next version and yes, this should be included in that 
release. Since 3.4.11 will probably be released together with 3.5.0 and 
@spmallette [just 
commented](https://issues.apache.org/jira/browse/TINKERPOP-2534?focusedCommentId=17302827=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17302827)
 on an expected release data for that version, I'm just adding his comment here 
again:
   
   > we had discussed the end of this month but I think it will dip into April 
at this point unfortunately.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Gremlin .NET driver ConnectionPool can remain without connections if server 
> is down for 1-2 minutes
> ---
>
> Key: TINKERPOP-2531
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2531
> Project: TinkerPop
>  Issue Type: Bug
>Affects Versions: 3.4.10
>Reporter: Radu Iviniciu
>Priority: Major
>
> We are using Gremlin .NET client to connect to AWS Neptune. 
> If the server is down for even just 1 - 2  minutes (E,.g. Instance reboot) 
> the connection pool can remain without any healthy connections permanently 
> even after the server comes back up again. 
> Tracked this down to here: 
> https://github.com/apache/tinkerpop/blob/60bfc90ce43567d609b1165989c1c74ce825109b/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs#L154
> If the server was not ready yet and the connections were not restored as part 
> of  the previous TryGetAvailableConnection then the connection pool will 
> remain empty indefinitely and all connections are dead even after the server 
> is back up. 
> Is this longer-ish downtime a use case the client should handle ? Or is it 
> expected behavior and we should let the caller handle the 
> ServerUnavailableException, potentially recreate the GremClient altogether ?  
> What do you guys think ? If this is something the client should handle I can 
> take a stab at it and put up a PR. 
> Thank you. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (TINKERPOP-2472) GraphBinary support in .NET

2021-03-17 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/TINKERPOP-2472?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17303341#comment-17303341
 ] 

ASF GitHub Bot commented on TINKERPOP-2472:
---

FlorianHockmann commented on pull request #1403:
URL: https://github.com/apache/tinkerpop/pull/1403#issuecomment-801037619


   > Please create a follow-on issue in JIRA for it and it can be some we 
return to later. 
   
   Done: https://issues.apache.org/jira/browse/TINKERPOP-2536
   
   > GraphSON supports metrics
   
   GraphSON does in general but I don't think that we have ever implemented 
support for them in Gremlin.Net. I just tried a `Profile()` traversal and get 
an exception: `Deserializer for "g:TraversalMetrics" not found`.
   
   > I presume we would add Upgrade Documentation and Reference Documentation 
adjustments when that happens?
   
   I would have added them now as GraphBinary is ready to be used with this PR 
from my side, but if you think that we definitely need support for metrics, 
then I can also add that first.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> GraphBinary support in .NET
> ---
>
> Key: TINKERPOP-2472
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2472
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: dotnet
>Affects Versions: 3.4.8
>Reporter: Florian Hockmann
>Priority: Major
>
> Support GraphBinary in Gremlin.Net.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (TINKERPOP-2536) Run Gherkin tests with GraphBinary and GraphSON

2021-03-17 Thread Florian Hockmann (Jira)
Florian Hockmann created TINKERPOP-2536:
---

 Summary: Run Gherkin tests with GraphBinary and GraphSON
 Key: TINKERPOP-2536
 URL: https://issues.apache.org/jira/browse/TINKERPOP-2536
 Project: TinkerPop
  Issue Type: Improvement
  Components: dotnet
Reporter: Florian Hockmann


TINKERPOP-2472 adds support for GraphBinary in Gremlin.Net but unfortunately 
the Gherkin tests still only run with GraphSON 3. We should extend them to be 
able to work on multiple serializers (at least GraphSON 3 and GraphBinary) so 
we can test the serializers with every build.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (TINKERPOP-2531) Gremlin .NET driver ConnectionPool can remain without connections if server is down for 1-2 minutes

2021-03-17 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/TINKERPOP-2531?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17303298#comment-17303298
 ] 

ASF GitHub Bot commented on TINKERPOP-2531:
---

spmallette commented on pull request #1404:
URL: https://github.com/apache/tinkerpop/pull/1404#issuecomment-800992406


   @FlorianHockmann please be sure to add a changelog entry on merge. 
   
   VOTE +1



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Gremlin .NET driver ConnectionPool can remain without connections if server 
> is down for 1-2 minutes
> ---
>
> Key: TINKERPOP-2531
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2531
> Project: TinkerPop
>  Issue Type: Bug
>Affects Versions: 3.4.10
>Reporter: Radu Iviniciu
>Priority: Major
>
> We are using Gremlin .NET client to connect to AWS Neptune. 
> If the server is down for even just 1 - 2  minutes (E,.g. Instance reboot) 
> the connection pool can remain without any healthy connections permanently 
> even after the server comes back up again. 
> Tracked this down to here: 
> https://github.com/apache/tinkerpop/blob/60bfc90ce43567d609b1165989c1c74ce825109b/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs#L154
> If the server was not ready yet and the connections were not restored as part 
> of  the previous TryGetAvailableConnection then the connection pool will 
> remain empty indefinitely and all connections are dead even after the server 
> is back up. 
> Is this longer-ish downtime a use case the client should handle ? Or is it 
> expected behavior and we should let the caller handle the 
> ServerUnavailableException, potentially recreate the GremClient altogether ?  
> What do you guys think ? If this is something the client should handle I can 
> take a stab at it and put up a PR. 
> Thank you. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)