Run cleanup of expired DB file locks to background job

[MorrisJobke]

• fixes Database based file locking creates many DELETE statements #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.

cc @LukasReschke @icewind1991 @PVince81 @karlitschek @DeepDiver1975 @rullzer

@karlitschek As discussed I would like to backport this to stable8.2 and stable9.

@cmonteroluque @karlitschek The best would be to have this in 8.2.3 and 9.0.0, but would be okay-ish for 8.2.4 and 9.0.1 too.

[mention-bot]

By analyzing the blame information on this pull request, we identified @icewind1991, @nickvergessen and @schiesbn to be potential reviewers

[MorrisJobke]

Steps to reproduce:

• use db based file locking
• create an entry in oc_files_lock with a TTL in the future (run date +%s on the command line and the TTL of the new entry needs bigger than this)
• run the cron.php and verify this background job has run (set the last execution before calling the cron.php to 0)
• the lock entry must still exist
• change the TTL to something that is smaller than the current unix timestamp
• run the cron.php and verify this background job has run (set the last execution before calling the cron.php to 0)
• the lock entry has to be removed

[icewind1991]

👍 looks good

[PVince81]

If not critical I'd suggest we put this in 9.0.1. We're already running out of time and this fix looks like it could use more time for testing.

[PVince81]

Ah right, but you mentionned 8.2.3... not sure...

[PVince81]

@MorrisJobke are there many big known setups that use DB locking ?

[MorrisJobke]

@MorrisJobke are there many big known setups that use DB locking ?

Currently it's only one, but 8.2.3 will be a release to which potential more instances will upgrade, because it is not a .0 or .1 release anymore. So we probably need a release note for this otherwise.

[karlitschek]

as discussed. backport is important. 👍

[ghost]

ok, we'll try to bring this in after the backport is confirmed (reviewed)

[MorrisJobke]

stable9: #22867

[MorrisJobke]

stable8.2: #22868

I tested both backports and they work just fine. I moved them in the next-maintenance milestone for now. If you feel comfortable with merging them for the 9.0 and 8.2.3 release feel free to do so. I would be more than happy 😃

[PVince81]

Tested, works 👍

[lock]

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.