mac os: leftover wal/shm files

[karlicoss]

Been running browserexport and noticed that it was leaving some wal files in the backup directory.

Tried running sqlite_backup tests and some actually fail.

FAILED tests/test_sqlite_backup.py::test_copy_to_another_file - AssertionError: assert {PosixPath('/....sqlite-wal')} == {PosixPath('/...0/db.sqlite')}
FAILED tests/test_sqlite_backup.py::test_backup_with_checkpoint - AssertionError: assert {PosixPath('/....sqlite-wal')} == {PosixPath('/...0/db.sqlite')}
FAILED tests/test_sqlite_backup.py::test_backup_without_checkpoint - AssertionError: assert {PosixPath('/....sqlite-wal')} == {PosixPath('/...0/db.sqlite')}
PASSED tests/test_sqlite_backup.py::test_open_asis
PASSED tests/test_sqlite_backup.py::test_do_copy
PASSED tests/test_sqlite_backup.py::test_do_immutable
PASSED tests/test_sqlite_backup.py::test_no_copy_use_tempdir
PASSED tests/test_sqlite_backup.py::test_do_copy_and_open
PASSED tests/test_sqlite_backup.py::test_database_doesnt_exist
PASSED tests/test_sqlite_backup.py::test_copy_retry_strict
PASSED tests/test_sqlite_backup.py::test_copy_different_source_and_dest
PASSED tests/test_threads.py::test_thread_wrapper_none
PASSED tests/test_threads.py::test_thread_wrapper_has
PASSED tests/test_threads.py::test_thread_raises

Will try to investigate and pehraps add a mac os CI pipeline, but for now just leaving it here

[purarue]

dont have access to my mac till early january, will take a look then

[karlicoss]

Interesting enough, seems that it doesn't remove wal even after checkpoint pragma.

Remembered out discussion on zulip: https://memex.zulipchat.com/#narrow/stream/279601-hpi/topic/atomically.20copying.20sqlite.20databases/near/270451281

Checked .dbconfig as suggested here, but I have no_ckpt_on_close off ... odd

[purarue]

Strange -- that is what I was expecting to be the issue

I have noticed dbs on Mac tend to be held open much longer - the files are much larger (both the Safari db and imessage db when I'm copying from), but I'm not sure if that's something custom in apple source code, some SQLite custom build flag or something else

[purarue]

updated the CI so at least it runs here, even if it fails: d2e4c41

[purarue]

hmm weird -- added macos/windows but it seems to pass on CI: https://github.com/seanbreckenridge/sqlite_backup/actions/runs/3723426152/jobs/6314926386

will give it a try locally when I have my mac machine on me

[karlicoss]

Huh, interesting! I guess I'll play with it on CI in tmate and try to find out if there is a difference in sqlite versions or something

[karlicoss]

So looked a bit more.

On my laptop, even something basic like this results in leftover WALs:

rm -f test.db* && sqlite3 test.db 'PRAGMA journal_mode=wal; CREATE TABLE test(a INTEGER);' && ls -al test.db*
wal
-rw-r--r--  1 karlicos  wheel   8192 Dec 20 00:22 test.db
-rw-r--r--  1 karlicos  wheel  32768 Dec 20 00:22 test.db-shm
-rw-r--r--  1 karlicos  wheel      0 Dec 20 00:22 test.db-wal

Whereas on CI there are no turds.

Locally I have:

$ which sqlite3
/usr/bin/sqlite3
$ /usr/bin/sqlite3 -version
3.39.4 2022-09-07 20:51:41 6bf7a2712125fdc4d559618e3fa3b4944f5a0d8f8a4ae21165610e153f77aapl

Interesting enough, sqlite3 on mac os CI uses Android version by default?? And it's different from the one in /usr/bin/sqlite3 🤯

bash-3.2$ which sqlite3
/Users/runner/Library/Android/sdk/platform-tools/sqlite3
bash-3.2$ sqlite3 -version
3.32.2 2021-07-12 15:00:17 bcd014c473794b09f61fbc0f4d9488365b023f16123b278dbbd49948c27calt2
bash-3.2$ /usr/bin/sqlite3 -version
3.32.3 2020-06-18 14:16:19 02c344aceaea0d177dd42e62c8541e3cab4a26c757ba33b3a31a43ccc7d4aapl

Also tried the brew version on CI

/usr/local/opt/sqlite3/bin/sqlite3 -version
3.40.0 2022-11-16 12:10:08 89c459e766ea7e9165d0beeb124708b955a4950d0f4792f457465d71b158d318

Same -- no turds. So I assume it's something with mac os rather than sqlite or its flag (.dbconfig is the same on both systems).

[karlicoss]

Played a bit more -- suspect it has to do with SQLITE_FCNTL_PERSIST_WAL. Looks like it's only exposed in C API via sqlite3_file_control method, but doesn't look like it's supported in sqlite python's API (or at least I haven't managed to find).

I tried alternative bindings which do expose it: https://github.com/rogerbinns/apsw

• first just check that wal behaves as expected on ungraceful shutdown (os._exit doesn't give connection a chance to cleanup)

#!/usr/bin/env python3
import ctypes
import apsw

db = apsw.Connection("test.db")

value = ctypes.c_int(0) # disable persistent WAL
assert db.filecontrol('main', apsw.SQLITE_FCNTL_PERSIST_WAL, ctypes.addressof(value))

db.execute("PRAGMA journal_mode=wal;")
db.execute("CREATE TABLE test(a INTEGER);")
db.execute("INSERT INTO test VALUES (1);")
db.execute("PRAGMA wal_checkpoint(TRUNCATE);")

import os
os._exit(0)

db.close()

$ rm -f test.db* && ./test.py && ls -al test.db*
-rw-r--r--  1 karlicos  wheel   8192 Dec 20 01:35 test.db
-rw-r--r--  1 karlicos  wheel  32768 Dec 20 01:35 test.db-shm
-rw-r--r--  1 karlicos  wheel      0 Dec 20 01:35 test.db-wal

as expected, WAL is empty, so checkpoint worked

• without os._exit, wal/shm are cleaned up -- as expected because we pass 0 as SQLITE_FCNTL_PERSIST_WAL.
• now if I change to value = ctypes.c_int(1), this enables persistent wal, and I end up with wal/shm even after a proper db.close() calll

$ rm -f test.db* && ./test.py && ls -al test.db*
-rw-r--r--  1 karlicos  wheel   8192 Dec 20 01:36 test.db
-rw-r--r--  1 karlicos  wheel  32768 Dec 20 01:36 test.db-shm
-rw-r--r--  1 karlicos  wheel      0 Dec 20 01:36 test.db-wal

So I suspect it has something to do with this? Although it's weird that python bindings and sqlite3 CLI behave in the same way, but perhaps they both include some OS specific settings headers 🤷

Not sure what's the best thing to do -- doesn't look like python's bindings are configurable so maybe the only way is to just forcefully remote WAL/SHM file after closing the connection (+ assert that wal is empty). Would be nice to make it conditional -- not sure on what though.

Maybe platform.mac_ver? On github actions it's ('11.7.2', ('', '', ''), 'x86_64'), whereas on my system it's ('13.0.1', ('', '', ''), 'arm64')

[karlicoss]

Alternatively :)

$ git diff
diff --git a/sqlite_backup/core.py b/sqlite_backup/core.py
index be47db4..0bb9307 100644
--- a/sqlite_backup/core.py
+++ b/sqlite_backup/core.py
@@ -243,6 +243,8 @@ def sqlite_backup(
             f"Running backup, from '{copy_from}' to '{destination or 'memory'}'"
         )
         with sqlite3.connect(copy_from, **sqlite_connect_kwargs) as conn:
+            if copy_use_tempdir:
+                conn.execute('PRAGMA journal_mode=DELETE;')
             conn.backup(target_connection, **sqlite_backup_kwargs)

         if destination is not None and wal_checkpoint:

this passes all tests on my laptop -- not sure if there is any issue with that I don't see? Somewhat surprising, because from documentation it should be the default

[purarue]

Ah thanks for investigating -- seems it was a flag, just not the one in familiar with

Ideally could have that SQLite version on the ci but don't know how possible that is, don't need to worry about that for now

Yeah I'd be fine with the pragma statement there

Was talking about it at dinner and my brother @AndrewSB is insisting a commit after the backup would fix this -- if backing up to a file (not to memory), could try that

[karlicoss]

Yeah, initially had no idea about the flag either -- reading wal.c from sqlite source code ended up way more fruitful than half an hour of googling haha

[karlicoss]

Hmm -- doesn't look like target_connection.commit() changes anything.

I'm tempted to call it a day and just merge my fix -- my only concern is that I don't really understand it now
I suggested this -- and it passes tests

         with sqlite3.connect(copy_from, **sqlite_connect_kwargs) as conn:
+            if copy_use_tempdir:
+                conn.execute('PRAGMA journal_mode=DELETE;')
             conn.backup(target_connection, **sqlite_backup_kwargs)

However I made a typo here -- makes more sense to use target_connection.execute('PRAGMA journal_mode=DELETE;'). Interesting enough it doesn't help 🤔
Interesting enough if I do this:

            conn.backup(target_connection, **sqlite_backup_kwargs)
            target_connection.execute('PRAGMA journal_mode=delete;')

, it cleans up the -wal file (for target database!), but still keeps -shm turd.

Either way, it works -- so not sure if it's worth overthinking it too much, especially considering it only applies to a temporary database (so can't really break anythingn).

[purarue]

Yeah, Im happy to merge anyways, just add a link above the line to this issue

[purarue]

Can close this if you're satisfied

[karlicoss]

also checked against browserexport, and no shmems anymore 🎉

[purarue]

Great, will publish a pip version and bump browserexport required version later tonight