Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix rare deadlock when using read/write locks #16169

Merged
merged 3 commits into from Aug 23, 2023

Conversation

erikjohnston
Copy link
Member

Turns out #16133 was incorrect, as the lock for the row being deleted is taken out before the trigger fires.

Let's revert that, and instead try and fix the race by only updating the _mode table if the deleted row's token matches. This way when only one lock at a time will attempt to update the _mode table on deletion. Similarly for insertion.

@erikjohnston erikjohnston marked this pull request as ready for review August 23, 2023 14:57
@erikjohnston erikjohnston requested a review from a team as a code owner August 23, 2023 14:57
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks reasonable but I said the same about the last.

Would be pretty nice if we could test it, but I'm not sure how tricky that would be...

AND token = OLD.token
FOR UPDATE;

IF NOT FOUND THEN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooi: does still still work as expected with PERFORM?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You make a good point there. I'll shove the returned row into a variable, as I can't see any obvious documentation

@erikjohnston
Copy link
Member Author

Would be pretty nice if we could test it, but I'm not sure how tricky that would be...

I haven't thought of a way of doing so :( I did try spawning and dropping lots of locks at once, but that didn't trigger the failure mode.

@erikjohnston erikjohnston merged commit 1827963 into develop Aug 23, 2023
13 checks passed
@erikjohnston erikjohnston deleted the erikj/fix_deadlock_attempt2 branch August 23, 2023 15:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants