Fix timeout on heartbeat with PostgreSQL (fixes #804) #809
Conversation
I could see in PostgreSQL logs that heartbeat operations were starting a transaction without closing them (either commit/rollback), leading into errors like: ``` process 14701 still waiting for ShareLock on transaction 3024907 after 1000.114 ms ```
| @@ -92,5 +94,7 @@ def ping(request): | |||
| except: | |||
| logger.exception("Heartbeat Failure") | |||
| return False | |||
| finally: | |||
| transaction.abort() | |||
There was a problem hiding this comment.
Don't you want to commit rather than abort?
There was a problem hiding this comment.
Why not... it will leave a useless trace in the DB, but we delete it on each call anyway.
Your call!
| @@ -17,6 +19,8 @@ def get_heartbeat(request): | |||
| def heartbeat_check(name, func): | |||
| status[name] = False | |||
| status[name] = func(request) | |||
| # Rollback any open transaction (will release table locks etc.) | |||
| transaction.abort() | |||
There was a problem hiding this comment.
Don't we want to do that at a upper level? I mean make sure the transaction have been canceled or committed each time a response have been sent?
| # Since the heartbeat checks run concurrently, their transactions | ||
| # overlap and might end in shared lock errors. By aborting here | ||
| # we clean-up the state on each heartbeat call instead of once at the | ||
| # end of the request. See bug Kinto/kinto#804 | ||
| transaction.abort() |
There was a problem hiding this comment.
Any idea why we abort rather than committing?
There was a problem hiding this comment.
The commit would have the same result (ie. ending the transaction), except that something would be left in the database between two calls.
Would you prefer ?
There was a problem hiding this comment.
Yes i guess it makes sense to make sure the database is writable.
|
r+ |
I could see in PostgreSQL logs that heartbeat operations were starting a transaction without closing them (either commit/rollback), leading into errors like: