tx_commit/1 doesn't appear to work, transactions generally wonky

[dotdotdotpaul]

Something seems very wrong with transactions. I think part of it is code, part of it might be Neo4j itself, but I'm not sure. I've forked the repo and done a little twiddling in transaction.ex, and then added to the transaction test to see if the main Neo4j connection can "see" the transactional changes before they're committed -- unfortunately, that seems to be the case.

Please take a look at https://github.com/dotdotdotpaul/neo4j_sips

One problem I noticed is that if you call tx_commit(conn) (with nothing else), the code would generate a server error during the commit, because it tries to execute a statement "" (empty string). I think my refactoring fixed that (using nil to indicate "no statements") but I still kept getting some odd behavior with regards to things persisting after a rollback...

So I expanded the test to check to see if the Neo4j.conn "sees" the changes made by the transactional conn -- and it does. That's concerning. But I also can't tell if that particular problem is due to Neo4j or to something in this library...

...Paul

[florinpatrascu]

Hi there - thank you for this report, I'll check again that part. What version of Neo4j, are you using for the tests?

[dotdotdotpaul]

2.3.2 Community

[florinpatrascu]

hmm, ok, I'll try that; I am using 3

[dotdotdotpaul]

I'll try updating to 3.x myself and see if that changes anything.

[florinpatrascu]

not sure if that would change anything?! The issue you found probably has nothing to do with the Neo4j.

[dotdotdotpaul]

Just tried with 3.0.3 and the same tests fail in the same way. I'm kinda HOPING it's just a problem with Neo4j.Sips, only because that would be a lot easier to fix. ;)

[florinpatrascu]

:) Now I need a reliable test, to make sure I can reproduce the issue you found.

[dotdotdotpaul]

Try the code in my fork? :)

[florinpatrascu]

sure, looking at https://github.com/dotdotdotpaul/neo4j_sips/blob/master/test/neo4j_sips_transaction_test.exs

[dotdotdotpaul]

That's the one. :)

[florinpatrascu]

This: "commit statements in an open transaction", is the one failing, right?!

[dotdotdotpaul]

Actually, they both fail at this point, at the point where I'm testing that the change made in the transaction is NOT visible to the "main connection" until the transaction is committed...

pclegg:~/work/neo4j_sips (master)$ mix test
test/neo4j_sips_transaction_test.exs:75: warning: variable results is unused
.....

  1) test rollback statements in an open transaction (Neo4j.Sips.Transaction.Test)
     test/neo4j_sips_transaction_test.exs:29
     Main connection should not be able to see transactional change
     stacktrace:
       test/neo4j_sips_transaction_test.exs:44



  2) test commit statements in an open transaction (Neo4j.Sips.Transaction.Test)
     test/neo4j_sips_transaction_test.exs:59
     Main connection should not be able to see transactional change
     stacktrace:
       test/neo4j_sips_transaction_test.exs:71

.......

Finished in 0.9 seconds (0.1s on load, 0.7s on tests)
14 tests, 2 failures

Randomized with seed 447555

[florinpatrascu]

I only imported the test/neo4j_sips_transaction_test.exs and didn't start changing my original code yet, I only have one test failing ATM, the one I mentioned above.

[dotdotdotpaul]

Oh, cool. That's a good sign. Maybe I flubbed something up in trying to fix that "invalid query" error when calling tx_commit(conn)... (which is what that failing test is testing).

[florinpatrascu]

This is what I have, so far:

~/alchemy_lab/neo4j_sips $ mix test test/neo4j_sips_transaction_test.exs 
test/neo4j_sips_transaction_test.exs:75: warning: variable results is unused
..

  1) test commit statements in an open transaction (Neo4j.Sips.Transaction.Test)
     test/neo4j_sips_transaction_test.exs:59
     Assertion with == failed
     code: Enum.count(results) == 1
     lhs:  0
     rhs:  1
     stacktrace:
       test/neo4j_sips_transaction_test.exs:80



Finished in 0.1 seconds (0.1s on load, 0.04s on tests)
3 tests, 1 failure 

[dotdotdotpaul]

Cool. I suspect the ultimate call to tx_commit(conn, "") is failing because the empty string is an invalid query. So when that actually gets called, the failure of the "empty string query" actually results in a rollback.

[florinpatrascu]

Elixir 1.2.6?

[dotdotdotpaul]

1.2.5, but hopefully that doesn't change much.

[florinpatrascu]

nailed it. Please pull the code from the tx_debug branch, it should work now. You suspected the driver and the server too, and you were right :) On my side, I had a design issue. The Query will "auto-commit" any query executed in an open transaction. Neo4j also has a "specific" behaviour that I was not aware of; the docs are not exactly clear sometimes. Committing an open transaction with no params, must be treated a bit different otherwise Neo4j will simply not commit :( In my case, when committing a Tx i.e. Neo4j.tx_commit(conn), the driver will send an empty statement to the server:

{
  "statements" : [ ]
}

and the server will basically not commit(close) the open transaction. Tricky to debug, but I found it.

Please have a look and check if your tests are passing now, I'll clean up the (Query) code tomorrow and merge the code to master. Taking a break now, and thanks so much for your findings. BTW, pls send any PR if need be, and I'll merge asap. Using your Tx tests from now on ;)

Cheers!

[dotdotdotpaul]

Whew, that's pretty crazy, glad I could help. If you've already pulled my Tx tests in, probably no need for a pull request at this point! ;)

[florinpatrascu]

code is in the master, and is was just published to hex.pm moments ago

Thanks again, Paul!