Fix could not serialize access due to concurrent DELETE from presence_stream

[realtyem]

• Fixes Could not serialize access due to concurrent DELETE from presence_stream #15827

I considered(and tested) using a Linearizer for this, I liked it's queueing behavior. However on further investigation, I found out that this type of situation has occurred before in Synapse history and was simply changing the isolation level to be slightly less strict than the default of REPEATABLE_READ. Using READ_COMMITTED allows the DELETE transaction to continue, even if it means the DELETE not seeing the concurrent transaction taking place to add a lower Stream ID(that this DELETE would otherwise have removed). Since either function will run again shortly, it will then subsequently remove the lower Stream ID entry and all will be well. The Postgres docs suggest that depending on the order of the transaction, it may still go ahead and DELETE the row anyways instead of ignoring it, which seems to have been the intended behavior.

... DELETE,... commands behave the same as SELECT in terms of searching for target rows: they will only find target rows that were committed as of the command start time. However, such a target row might have already been updated (or deleted or locked) by another concurrent transaction by the time it is found. In this case, the would-be updater will wait for the first updating transaction to commit or roll back (if it is still in progress). If the first updater rolls back, then its effects are negated and the second updater can proceed with updating the originally found row. If the first updater commits, the second updater will ignore the row if the first updater deleted it, otherwise it will attempt to apply its operation to the updated version of the row. The search condition of the command (the WHERE clause) is re-evaluated to see if the updated version of the row still matches the search condition. If so, the second updater proceeds with its operation using the updated version of the row.

Related docs and PRs/Issues:

• Use READ COMMITTED isolation level when inserting read receipts #12957
• Use READ COMMITTED isolation level when purging rooms #12942
• Room deletion (shutdown) fail in a constant loop due to non-serializable access caused by PostgreSQL isolation levels #10294
• stop using REPEATABLE READ everywhere #11567
• Postgres docs on isolation level

Note: This has no effect for SQLite usage, as attempt_to_set_isolation_level()SQLite version says everything is set to SERIALIZABLE regardless.

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file.
• Pull request includes a sign off
• Code style is correct
  (run the linters)

Signed-off-by: Jason Little [email protected]

EDIT: Added some pretty pictures

[realtyem]

Actually, let's hold off on this for now. I think I may have found the place that was causing this behavior. While I investigate how hard this will be to untangle, I'll leave this in draft.

[realtyem]

Let's go ahead with this, the angle I was looking at will not solve this on it's own.

[reivilibre]

can't see why this would pose any problems given the txn that is being run