search

nikita-volkov / hasql-transaction

A composable abstraction over retriable transactions for Hasql
http://hackage.haskell.org/package/hasql-transaction
MIT License
12 stars 15 forks source link

WARNING: there is no transaction in progress #13

Closed domenkozar closed 4 years ago

domenkozar commented 4 years ago

I'm not yet sure how to debug this, but I'm getting a bunch of warnings on stderr of Haskell app.

My understanding is that if hasql-pool and hasql-transaction worked correctly, these shouldn't happen.

nikita-volkov commented 4 years ago

Can you isolate the issue? We need a reproduction.

domenkozar commented 4 years ago

I'll try to reproduce it.

domenkozar commented 4 years ago

I know that it happens under HTS.transaction HTS.Serializable HTS.Write and under heavier load, that's about what I can say for now.

nikita-volkov commented 4 years ago

Can you test it on version 0.10? It's a rewrite, which is a candidate for next major release. As a heads up, the API is different.

As a side note, your review of the candidate API, will be useful.

domenkozar commented 4 years ago

@nikita-volkov will test - probably tomorrow. I wonder though, why is there 0.10 and 0.9, while 1 was released a while ago?

nikita-volkov commented 4 years ago

I use the convention of making experimental releases in the 0.* version space.

domenkozar commented 4 years ago

Rewrite doesn't have monad instance, which I need as one transaction input depends on another output.

nikita-volkov commented 4 years ago

Yes, there's no Monad for reasons explained in the docs, but it has Selective, which should be enough in many cases. It also now allows you to lift sessions into it, and sessions are monads.

nikita-volkov commented 4 years ago

Check out this tutorial:

https://github.com/nikita-volkov/hasql-tutorial1/blob/master/library/HasqlTutorial1/Transaction.hs

Particularly, the difference between register and register'.

domenkozar commented 4 years ago

Those are not the same, register runs both queries in the same transaction, where as register' runs them in separate, which may lead to inconsistencies.

domenkozar commented 4 years ago

I found the reason for this warning.

From our logs:

2019-11-25 13:02:35.198 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 LOG:  execute 8: COMMIT
2019-11-25 13:02:35.198 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 ERROR:  could not serialize access due to read/write dependencies among transactions
2019-11-25 13:02:35.198 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 DETAIL:  Reason code: Canceled on commit attempt with conflict in from prepared pivot.
2019-11-25 13:02:35.198 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 HINT:  The transaction might succeed if retried.
2019-11-25 13:02:35.198 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 STATEMENT:  COMMIT
2019-11-25 13:02:35.199 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 LOG:  execute 12: ABORT
2019-11-25 13:02:35.199 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 WARNING:  there is no transaction in progress

Turns out if COMMIT fails, that's already considered as an ABORT according to postgresql. So issuing an ABORT is redundant.

nikita-volkov commented 4 years ago

Those are not the same, register runs both queries in the same transaction, where as register' runs them in separate, which may lead to inconsistencies.

Both these functions execute as a single transaction. That's the whole point of composition of transactions.

nikita-volkov commented 4 years ago

I found the reason for this warning.

From our logs:

2019-11-25 13:02:35.198 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 LOG:  execute 8: COMMIT
2019-11-25 13:02:35.198 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 ERROR:  could not serialize access due to read/write dependencies among transactions
2019-11-25 13:02:35.198 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 DETAIL:  Reason code: Canceled on commit attempt with conflict in from prepared pivot.
2019-11-25 13:02:35.198 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 HINT:  The transaction might succeed if retried.
2019-11-25 13:02:35.198 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 STATEMENT:  COMMIT
2019-11-25 13:02:35.199 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 LOG:  execute 12: ABORT
2019-11-25 13:02:35.199 GMT [19304] cachix [unknown] cachix [local] 5ddbd0bb.4b68 WARNING:  there is no transaction in progress

Turns out if COMMIT fails, that's already considered as an ABORT according to postgresql. So issuing an ABORT is redundant.

Thanks for your reproduction. I'll look into this.

nikita-volkov commented 4 years ago

Turns out if COMMIT fails, that's already considered as an ABORT according to postgresql. So issuing an ABORT is redundant.

Yes, it looks like you're correct in your diagnosis. Have you found any reference in the official docs on this? I'm struggling to find it.

domenkozar commented 4 years ago

I've only found https://www.postgresql.org/message-id/27ade5280909291219v5ce78747pcb426e5444cb506a%40mail.gmail.com, but probably you can check by creating a query that fails and wrap it into an transaction.

nikita-volkov commented 4 years ago

Okay. I've put together a patch, which should fix the issue. Can you try it? It's in commit be077c0c5fe9978b89b35a4c8d03491cffcea233

domenkozar commented 4 years ago

Going to run it for a few days and report back if there are no more warnings (they used to appear daily).

domenkozar commented 4 years ago

There were no warnings since, so it's safe to say it's fixed.

nikita-volkov commented 4 years ago

Hopefully no other bugs were introduced :)

Okay. Thanks! I'll merge the patch.

nikita-volkov commented 4 years ago

The patch is merged in release 1.0.0.1

domenkozar commented 4 years ago

Thanks!