Reduce Postgres transaction isolation level #1860
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was able to reproduce #1856 by replaying the "concurrent channel
updates" test with hardcoded additional delays in the database code. It
was due to a conflict between
addOrUpdateChannel
andupdateChannelMetaTimestampColumn
. The two calls run in parallel andthe latter completed before the former, causing it to fail. Reducing
the isolation level makes the problem disappear.
We reduce the transaction isolation level from
SERIALIZABLE
toREAD_COMMITTED
. Note that [*]:I'm not sure why we were using a stricter isolation level than the
default one, since we only use very basic queries. Doc does say that:
To make sure this didn't cause regression withe the locking
mechanism, I wrote an additional test specifically on the
withLock
method.
Here is what the doc says on the
INSERT ON CONFLICT DO UPDATE
statement, which we use for
addOrUpdateChannel
:In the scenario described above, the
addOrUpdate
will update the rowwhich timestamp was updated in parallel by the
updateChannelMetaTimestampColumn
, and it's exactly what we want.Fixes #1856.
[*] https://www.postgresql.org/docs/13/transaction-iso.html