-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[stable8.2] Run cleanup of expired DB file locks to background job #22868
Conversation
* fixes #22819 The old way fired a DELETE statement on each destruction of the DBLockingProvider. Which could cause a lot of queries. It's enough to run this every 5 minutes in a background job, which in the end could result in file locks that exists 5 minutes longer - in the worst case and for not properly released locks. This makes the DB based locking a lot more performant and could result in a similar performance to the Redis based locking provider.
By analyzing the blame information on this pull request, we identified @icewind1991, @nickvergessen and @schiesbn to be potential reviewers |
I tested this with an upgrade from 8.2.2 and it works just fine. |
👍 |
Tested, works 👍 |
Setting to 8.2.3 as discussed with @cmonteroluque |
Tested, looks good 👍 |
@cmonteroluque The failing test could be ignored for stable8.2 |
[stable8.2] Run cleanup of expired DB file locks to background job
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
The old way fired a DELETE statement on each destruction of the
DBLockingProvider. Which could cause a lot of queries. It's enough
to run this every 5 minutes in a background job, which in the end
could result in file locks that exists 5 minutes longer - in the
worst case and for not properly released locks.
This makes the DB based locking a lot more performant and could
result in a similar performance to the Redis based locking provider.
Backport of #22865
General backport approval is in #22865 (comment)
@cmonteroluque @PVince81 @karlitschek I just put this into 8.2.4 for now. Feel free to merge this to 8.2.3 if this is fine for you and it is tested properly.
cc @LukasReschke @icewind1991 @DeepDiver1975 @rullzer