Skip to content
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

SQLite does not remove *-wal and *-shm files when using a DatabasePool #739

Closed
mallman opened this issue Mar 25, 2020 · 14 comments
Closed

Comments

@mallman
Copy link
Collaborator

mallman commented Mar 25, 2020

What did you do?

Precondition

You have a SQLite database database.db with a table named "countMe".

Steps to reproduce

main.swift:

import GRDBCustomSQLite

var pool: DatabasePool! = try DatabasePool(path: "database.db", configuration: Configuration())
try pool.read { db in
  return try Row.fetchOne(db, sql: "select count(1) from countMe")
}
pool = nil

What did you expect to happen?

There is no library.db-wal or library.db-shm after the program exits.

What happened instead?

The files library.db-wal and library.db-shm exist after the program exits.

Environment

GRDB flavor(s): GRDBCustomSQLite
GRDB version: git tag v4.12.0
Installation method: Xcode build
Xcode version: 11.4 (11E146)
Swift version: 5.2
Platform(s) running GRDB: macOS
macOS version running Xcode: 10.15.4 (19E266)

Notes

SQLite appears to be sensitive to the order in which connections are closed. As-is, GRDB/Swift opens the pool writer first, then the reader pool. When it deinits, it closes the writer first and the reader connections second. If I add a line

readerPool = nil

to DatabasePool.deinit then SQLite deletes the wal and shm files as expected. Not sure this is the best fix, but it's what I'm going with for now.

@groue
Copy link
Owner

groue commented Mar 26, 2020

Hello @mallman,

The last conversation about the wal and shm files was #418.

My current position is:

  1. The wal and shm files do not harm. Do they?
  2. It's impossible to reliably remove them when a database pool deallocates, due to eventual external connections, zombie SQLite connections, application crashes, and kill -9. This means that all applications must be prepared for the presence of those files anyway.

If you witness that closing reader connections first helps, I'll favorably consider a pull request, though. After all, you may use GRDB with WAL databases that you do not own. But please consider adjusting your expectations as well. I personally wrap WAL databases in their dedicated directory, or a document bundle: this has other advantages.

@groue groue added the question label Mar 26, 2020
@mallman
Copy link
Collaborator Author

mallman commented Mar 26, 2020

Hi @groue,

The reason I'm concerned is that—with the given behavior—I cannot (just looking at the contents of the filesystem) identify whether a SQLite database has been closed cleanly or not. Additionally, one of the benefits of SQLite is that a (unopened) database is encapsulated completely within a single file. Having the wal and shm files around complicates matters and introduces uncertainty. I'd like to expect that having those files around means either an unclean shutdown or an open connection so an external backup process can proceed appropriately.

From #418 and from my own interpretation of the SQLite documentation, I cannot see why the order in which the database handles are closed is relevant to SQLite's claim to remove the wal and shm files on a clean exit. Perhaps that is a bug in SQLite, or a point of confusion in the documentation. I will try to get clarity on the matter from the SQLite side. Would you be willing to keep this issue open for a while while I gather more information about SQLite?

Thank you.

@mallman
Copy link
Collaborator Author

mallman commented Mar 26, 2020

Oh and one other thing I thought of, which was relevant to my own development efforts. For the longest time I assumed I was not properly releasing my application's references to my db pool because of the presence of the "dangling" wal and shm files. That led to hours of debugging, experimentation and frustration. Therefore, I think another practical reason to remove these files on a clean exit (as documented by SQLite) is to provide assurance to an app developer that their app is properly releasing all of its connections on exit.

@mallman
Copy link
Collaborator Author

mallman commented Mar 26, 2020

I ran your CLI example from #418 with two different versions of SQLite and got two different results. To wit:

[msa@dandy ~]$ rm /tmp/test.db*
rm: /tmp/test.db*: No such file or directory
[msa@dandy ~]$ /usr/bin/sqlite3 /tmp/test.db
SQLite version 3.28.0 2019-04-15 14:49:49
Enter ".help" for usage hints.
sqlite> PRAGMA journal_mode = WAL;
wal
sqlite> CREATE TABLE t(a);
sqlite> .exit
[msa@dandy ~]$ ls /tmp/test.db*
/tmp/test.db		/tmp/test.db-shm	/tmp/test.db-wal
[msa@dandy ~]$ rm /tmp/test.db*
[msa@dandy ~]$ sqlite3 /tmp/test.db
SQLite version 3.31.1 2020-01-27 19:55:54
Enter ".help" for usage hints.
sqlite> PRAGMA journal_mode = WAL;
wal
sqlite> CREATE TABLE t(a);
sqlite> .exit
[msa@dandy ~]$ ls /tmp/test.db*
/tmp/test.db
[msa@dandy ~]$ 

The take away here being that the stock version of SQLite that comes with macOS 10.15.4 does not clean up the wal and shm files. However, a more recent 3.31.1 build does. This raises my suspicion that this was a subtle bug with some versions of SQLite dating before 3.31.1. However, let me try to get an authoritative answer to resolve this issue definitively.

@groue
Copy link
Owner

groue commented Mar 26, 2020

@mallman, it's a delight to read your sensible answers. I'll sure keep this issue open. Please come back with new information, and ask for help if needed!

@mallman
Copy link
Collaborator Author

mallman commented Mar 26, 2020

I've just submitted a message to the SQLite forum (awaiting moderator approval). I've made an additional finding: a simple cc *.c *.h build of the SQLite source amalgamation for 3.28.0 does actually remove the wal and shm files in your test case. So the issue isn't endemic to 3.28.0, per se. It seems that some build option or maybe even some source customization with the default macOS build is causing the unexpected behavior.

@groue
Copy link
Owner

groue commented Mar 26, 2020

the SQLite forum

I didn't know the mailing list had been deprecated! https://sqlite.org/forum/forumpost/1fdfc1a0e7 👍

@groue
Copy link
Owner

groue commented Mar 27, 2020

So, what do you suggest, @mallman?

Do you feel like submitting a pull request with this extra readerPool = nil in DatabasePool.deinit?

Do you think this pull request should contain a test which checks for -shm and -wal file deletion in the simple cases, as a guarantee against regressions? Or do you think that this test would be flaky (in which case I don't want that we add it). If you feel confident, then it should be added to DatabasePoolReleaseMemoryTests.swift.

In any case, we should not extend or alter the documentation with any promise about the -shm and -wal files. Being a better SQLite citizen does not change the "official position": all applications must be prepared for the presence of those files anyway.

@mallman
Copy link
Collaborator Author

mallman commented Mar 27, 2020

Hi @groue.

So, what do you suggest, @mallman?

I think this is a bug in the build of SQLite that comes with macOS. There is nothing wrong with the way GRDB is closing connections, so I suggest we leave it as-is. While a workaround for an underlying problem is often practical and necessary, my impression is the impact of this bug is such that on balance it's better to keep the GRDB code base clean rather than incorporate a workaround.

I will keep my own private workaround for now, while I hope to make progress in identifying where the underlying issue creeps in. Eventually we may see a fix in the SQLite deployed with macOS.

Cheers!

@mallman mallman closed this as completed Mar 28, 2020
@groue
Copy link
Owner

groue commented Mar 28, 2020

All right, @mallman. Thank you for your dedication at figuring those things out!

cc @scottandrew

@pmanna
Copy link

pmanna commented Apr 24, 2020

Just to add to the discussion (no pretence of solution), there's a similar problem I've been experiencing: a bit of foreword, though…
We have an app suite, that relies of one reference app that maintains the common DB (a few million records) current, and other apps access, read-only, the tables needed for their features.

The obvious solution is to have an App Group, and that's what we've done: however, without the modification suggested above, the WAL file is not only never deleted, but grows at every launch, and it being a 400 MB DB, it gets quickly into the GB (yes, on macOS AND iOS, no difference there).
The readerPool = nil suggestion works, if the DatabasePool is explicitly set to nil and is then deallocated at the end, but it doesn't delete the WAL, it leaves it there at 0 bytes, so it's good enough for me… If it doesn't go in any official future release, I'll have to keep adding it!

@groue
Copy link
Owner

groue commented Apr 24, 2020

Hello @pmanna, and thank you!

In the current state of my knowledge about WAL, and the experiences reported so far, it seems to me that closing readers before the writer (readerPool = nil) does not harm, and even actually helps. I don't understand why, but complete understanding is not always necessary to proceed, fortunately :-)

I'll thus add it in the next GRDB release 👍

@groue groue reopened this Apr 24, 2020
@groue groue added this to the GRDB 5 milestone May 2, 2020
@groue groue closed this as completed in 399766f May 2, 2020
@groue
Copy link
Owner

groue commented May 3, 2020

Done in GRDB 5.0.0-beta

@groue
Copy link
Owner

groue commented May 6, 2020

According to recently acquired information, the -wal and -shm files are deleted when a database connection closes unless the SQLITE_FCNTL_PERSIST_WAL flag is set. I believe the surprising behavior of SQLite 3.28.0 on macOS is due to this flag, which is set by default.

See #771 (comment) for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants