Repository navigation
fix: presence stream stalling intermittently - #20090
Conversation
4ded8fa to
70787c6
Compare
70787c6 to
97ff706
Compare
MadLittleMods
added
A-Presence
A-Workers
sandhose
left a comment
sandhose
left a comment
There was a problem hiding this comment.
That is quite an interesting fix, thanks a lot for digging into that!
| _load, | ||
| db_autocommit=True, | ||
| ) | ||
| except BaseException: |
There was a problem hiding this comment.
This was a bit weird to me at first, why you would catch BaseException instead of Exception. Turns out, one thing we are interested is catching GeneratorExit, which happens in case a paused coroutine gets garbage collected. Could you just stick a comment why we're catching BaseException and not just Exception?
There was a problem hiding this comment.
done in the last commit
| id_gen.get_current_token_for_writer("master"), | ||
| 7, | ||
| "presence stream position is wedged by the cancelled allocation", | ||
| ) |
There was a problem hiding this comment.
I think this should be exactly 9, right? As there's the abandoned stream ID + the successful write. Can we assert that instead of just "greater than 7"?
There was a problem hiding this comment.
right, done in the last commit
comment on why we catch `BaseException` instead of `Exception`)
|
Thanks a lot for the quick review @sandhose ! |
|
The failing complement test is a flake @sandhose |
This is a fix for presence updates silently stalling when a
/syncrequest is cancelled mid-write, causing a stream ID to be leaked into_unfinished_idsand permanently pinning the persisted stream position.Fixes #19800
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.