Try to prevent possible deadlock in archive invalidator #15603
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tsteur
added
not-in-changelog
diosmosis
reviewed
Feb 20, 2020
core/Archive/ArchiveInvalidator.php
Outdated
| // we want to reduce number of rows scanned and only delete specific primary key | ||
| $keys = Option::getLike($id . '%'); | ||
| foreach ( $keys as $key => $val) { | ||
| Option::delete($key); |
There was a problem hiding this comment.
Would it make sense to do a DELETE FROM option WHERE option_name IN (...) so it's just one query?
There was a problem hiding this comment.
@diosmosis was thinking about it but then figured it's easier to just reuse existing code for now. Not sure it makes a big difference?
There was a problem hiding this comment.
I guess it depends on how many invalidated dates there can usually be at one time for a single site.
There was a problem hiding this comment.
Shouldn't be too many since we regenerate the tracker cache once the flag is written to avoid another request to set this flag? could be a few if there were many requests in parallel maybe. I'll try to have a look but don't really have time.
jonasgrilleres
pushed a commit
to 1024pix/pix-analytics
that referenced
this pull request
Sep 22, 2020
…5603) * Try to prevent possible deadlock in archive invalidator * use one query to delete all at once
jbuget
pushed a commit
to 1024pix/pix-analytics
that referenced
this pull request
Sep 26, 2020
…5603) * Try to prevent possible deadlock in archive invalidator * use one query to delete all at once
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See http://mitchdickinson.com/mysql-innodb-row-locking-in-delete/
The idea is that if we delete by primary key, then no other rows will be locked thus avoiding the deadlock hopefully. For deadlock info see #15545 (comment) and below.
Checked other Option::deleteLike but they seem fine.
fix #15545
Be great if that could make it into 3.13.3 since we would likely be facing these issues a lot too.
I haven't tested if it actually prevents the deadlock but hoping so. We will otherwise need to reopen #15545 if it doesn't.
Background: When we set an option, we set basically
$id_$processId. So this should prevent the lock when we delete the key directly by primary key since the appendix_$processIdshould be different and not prevent the insertion of another key.