-
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
using a db transaction when scanning (external) files with sqlite causes database locked errors #11391
Comments
@butonic, is this sqlite only? |
@craigpg yes, sqlite only |
Disabling transitions in the scanner would be a solution but it would cause a massive performance panelty |
@icewind1991 Why does the transaction improve the performance? I´m not sure I understand the reason by reading the code. |
Without the transaction scanning 10000 files makes 10000 individual updates, auto committing them. Which in turn causes the affected indexes to be updated - which is slow on all DBMS, but especially on sqlite and lower end hardware. (more details on sqlite insert performance in http://stackoverflow.com/questions/1711631/how-do-i-improve-insert-per-second-performance-of-sqlite) |
It would be great to measure this. In my experience smaller atomic updates are scale better because they can reordered and parallelized. This is especially interesting if you have a DB cluster. |
When I first introduced the transactions in the scanner it gave a 3x-5x performance increase |
But we have to make sure that it´s not locking the DB for a long time and triggering other bugs |
Yes, transactions speed up sqlite tremendously. Actually, write and update operations should not happen without transactions because they are way too slow. Same experience on the client. |
I have no idea how the file scanner or server works, just my 0.005 BTC cent: Can you guys code it in a way that it uses a transaction but commits every 200msec or so? However that still might be problematic because other concurrent requests (using https://sqlite.org/c3ref/busy_timeout.html or whatever you're using on the server, that's what we use in the client) might not suceed even when you're "between transactions". Maybe the file scanner could be "sleeping" not-in-any-transaction or so for some time to give other concurrent requests a chance to do something with the DB? (not relevant with real DB, only for sqlite3) |
Can we do smaller transactions to not run into a lock timeout? |
Does this problem only affect SQLlite? @dragotin referred this case in owncloud/client#2378 but I have it with MySQL. |
Pull request that adds some closeCursors: #14329 |
@PVince81 @icewind1991 Isn't this solved by high level file locking that was enabled in 8.2+? |
Such issues haven't been reported since a while, so it might be solved by locking, yes. |
Okay let's close this here and maybe reopen if this appears again. |
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. |
I assume the database transaction used to commit updates of the file scanning process in batches causes an sqlite database to lock for extended periods of time, leading to the
database locked error
when another request comes in. With preview generation, background jobs, hooks, search lucene and maybe sync clients coming along that is very likely to happen. The chance that a lock is in place increases with the latency of file scans, meaning scanning a remote files_external storage, such as amazon s3 is highly likely to produce the db lock.for reference, a little history:
16 Jan 2013 @icewind1991 adding a transaction to improve scan performance 6871a15
3 Nov 2013 @jancborchardt the original issue #5680
21 Aug 2014 @PVince81 assuming an open db cursor to be the issue #10566
8 Sep 2014 @icewind1991 making the transaction contain even more updates in master 644755d
30 Sep 2014 @butonic being able to reproduce the issue #11280 (comment)
1 Oct 2014 @jancborchardt confirming he was likely using files_external with sqlite #11280 (comment)
With 644755d has added a flag to the scanner that we could use to do without transactions on sqlite. AFAICT atomicity is not the reason for the transaction. Opinions @DeepDiver1975 @karlitschek @schiesbn @icewind1991
The text was updated successfully, but these errors were encountered: