feature request: Neptune transaction support

[tsanton]

Is there an existing issue for this?

• I have searched the existing issues

Feature description

Basic transactions were supported in Tinkerpop 3.7.0 by introducing a ´TinkerTransactionGraph´ class. The config for it can be found here.
I'm hoping it's possible for Localstack Neptune to support transactions.

🧑‍💻 Implementation

I know AWS support Gremlin 3.6.2 <= x <= 3.6.5 at the moment, and that Localstack is running a Gremlin Server behind the scenes. In terms of implementation I'm wondering if it's possible to implement the server config that's linked above by copying the 3.7.X jar and pointing to that transaction class? It would be extremely nice to be able to test locally with transactions, even when limited to single thread processing.

Anything else?

No response

[localstack-bot]

Welcome to LocalStack! Thanks for reporting your first issue and our team will be working towards fixing the issue for you or reach out for more background information. We recommend joining our Slack Community for real-time help and drop a message to LocalStack Pro Support if you are a Pro user! If you are willing to contribute towards fixing this issue, please have a look at our contributing guidelines and our contributing guide.

[cloutierMat]

Hi @tsanton,

I have looked into the issue. While there appears to have been more changes to the overall gremlin server that makes the simple lifting of a couple .jar files more complicated than it seems, there might be another solution that could work in the interim.

We could create a configuration variable like NEPTUNE_ENABLE_TRANSACTION=1 that would essentially make localstack disregard the Neptune engine version provided at creation and use 3.7 instead. This would have the added benefit of also adding support for property(set, "prop1", "prop2").

Looking at the release notes, I am not seeing any major deprecation going from 3.6 to 3.7. The other major difference is listed under Properties on Elements and seems to have a configurable work around.

Let me know if you see any other complications I am missing.

[tsanton]

Hi @cloutierMat and thank you for your expedient reply!

Personally I would love that solution. Though I'm not a Graph expert, I'd say I'm a layman and an enthusiast. With those restrictions in mind I can think of one potential issue: though deprecations are not a major issue, improvements on existing functionality from 3.6.x to 3.7.x can present behaviour problems (specifically when it comes to MergeV and MergeE). This GitHub issue explains one potential pitfall in depth. In short it's related to the option of specifying property cardinality during merge (can do in 3.7, not possible in 3.6).

If we implement the solution you suggest I would imagine the Localstack MergeE/MergeV behaviour with property cardinality would be 👍, whereas if you fired this at AWS Neptune you'd get a solid 👎. In my view I much prefer the (single thread) transaction support with a LocalStack note "FYI: Do not...." over the non-transactional server as is. Further I'm willing to bet a beer and a nacho (if you're ever somewhere in mid to north Europe) that Neptune 1.3.2.0 (due in x-3? months) will support >=3.7.x so this sort of ameliorates the issue. (Even)Further AWS Neptune docs clearly state: "[test your upgrades on real life instances before you roll out your new bundle of joy]" which gives us some leeway when implementing this behaviour change.

Thanks for you time and support @cloutierMat!

/T

[cloutierMat]

Thanks for pointing that out! I will keep an eye for it, and document my findings in the docs.

[cloutierMat]

Hi @tsanton,

The feature has now been implemented and you can pull the latest docker image to start using it. Information on how to enable it is in our Documentation.

When creating your Neptune instance you should see the following message in the logs indicating that transactions are enabled.

localstack-samples  | 2024-05-30T20:53:35.809  WARN --- [-functhread4] l.s.neptune.packages       : NEPTUNE_ENABLE_TRANSACTION flag is set. Ignoring 'engine-version' for version '3.4.11' and installing: '3.7.2'

As far as the MergeV/MergeE behaviour, from the few samples I have tried, 3.6.2 queries seem to be working as expected. Let us know how it goes.

[tsanton]

Hi @cloutierMat, and thanks for the swift implementation!

I'll get on to refactoring my fixture over the weekend/early next week. I'll get back to you with the results when that is done.

Have a great weekend when that time comes :)

/T

[tsanton]

Hi again @cloutierMat!

I've now had the time to look over it.
In short I have 328 tests of whom 326 came out green after swapping out the container and enabling transactions.
From these two failing tests (of whom one should fail) I've found one potential issue and one behaviour I'm not certain about.

Firstly the potential issue.

I have two verticies (V1 and V2) joined by an edge (LinkedE): V1 -> LinkedE -> V2.

This LinkedE has one property: Active (T/F). This property is being updated depending on, you guessed it: the relationship being active or not. The issue here, to me, seems to be the default cardinality. Now when I run the update test I get a failing test and I query out this result (edge properties):

{
          "key": "Active",
          "value": true,
          "metaPropertyOf": null,
          "id": "74aca9b1-efd3-4a9f-b608-4da20ece9321",
          "cardinality": "list",
 },
{
          "key": "Active",
          "value": false,
          "metaPropertyOf": null,
          "id": "405cee5c-3efc-4f83-8384-e959e1a87064",
          "cardinality": "list",
},

Though this docs is for vertex cardinality, is states that only single or set is supported. I think it's safe to assume that it would be the same for edges?

Unfortuntely I'm only in dev so I don't have neptune instance up just yet so I can't verify this.

The next "odd" behaviour is related to "implicit" transactions.
I had a test fail that previously passed: MergeV then MergeE but otherV does not exist, and I wanted all to fail and rollback. This is indeed the behaviour I'm seeing now, but it was happening before I set SupportsTransaction() to return true.

var g = await traversalFactory.GetClient(tenantId);
var tx = g.Tx();
if (traversalFactory.SupportsTransaction()) g = tx.Begin();
........
if (hasTx && tx is not null)
{
    await query.Promise(traversal => traversal.Iterate(), ct);
    await tx.CommitAsync(ct);
}
else
{
    await query.Promise(traversal => traversal.Iterate(), ct);
}

With other words it seems that an entire batch of steps is being given the transactional treatment (i.e. all or nothing). I'm then guessing that the explisit transaction usage is for multiple iterates? Further: is this in accordance with the behaviour of Neptune?

[cloutierMat]

Hi @tsanton,

Awesome that most of your tests are passing now 🔥 🚀

I am a little confused by the errors you are seeing and I would love for you to give me more details on your setup and sample queries to reproduce. But here are my thoughts for the 2 points you bring up.

Edge Property Cardinality

I am not sure how you could get a list Cardinality on an Edge property as it doesn't seem to be supported by the Apache Gremlin language or by AWS Neptune.

See Apache Docs. Point two states For vertices, a cardinality can be provided for vertex properties.. Both implementations seem to only support Single and no metaproperties can be added.

Please provide a sample code as to how you are achieving this! 😄

Implicit Transaction

Let me know if I understand you correctly. you are performing something like.

t = g.add_v("person").as_("p")
t.add_e("knows").from_("p").to("invalidId")
result = t.iterate()

The expected behaviour is that the user will not be created as each traversal is creating a transaction where all of the individual steps must resolve. A traversal will involve all the steps before a terminal step.

So transaction are only making a difference when multiple terminal steps have occurred in the transaction. See last example on this page.