[experiment] add alternative wasm sqlite3 implementation available via build-tag

[NyaaaWhatsUpDoc]

Description

• wasm sqlite3 can be accessed via wasmsqlite3 build tag
• moves sqlite driver wrapping to separate subpkg, same needs to be done for postgres eventually
• removes the sqlite3 interrupt workaround in the error processing (no longer needed)
• removes the retryBusy() handler from the modernc.org/sqlite wrapper, again no longer seems to be needed (maybe related to the custom modernc.org/sqlite branch we're on)

Checklist

• I/we have read the GoToSocial contribution guidelines.
• I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
• I/we have not leveraged AI to create the proposed changes.
• I/we have performed a self-review of added code.
• I/we have written code that is legible and maintainable by others.
• I/we have commented the added code, particularly in hard-to-understand areas.
• I/we have made any necessary changes to documentation.
• I/we have added tests that cover new code.
• I/we have run tests and they pass locally with the changes.
• I/we have run go fmt ./... and golangci-lint run.

[tsmethurst]

I would suggest that we keep the retry on busy handling, as it is still applicable to us: https://www.sqlite.org/wal.html#sometimes_queries_return_sqlite_busy_in_wal_mode

Also, we do let clients choose whether to use wal mode or not, and if they don't use wal mode for whatever reason, the busy handler will be much more necessary.

[daenney]

Also, we do let clients choose whether to use wal mode or not, and if they don't use wal mode for whatever reason, the busy handler will be much more necessary.

We should probably change that honestly. Letting clients chose the mode isn't really necessary, unless there's some evidence of WAL performing worse for people. But I suspect the evidence goes in the other direction, and it limits the amount of things we have to think about when someone reports a database performance issue.

Since we have the transpile or the WASM build we can rely on WAL always being available no matter the SQLite version any distro ships.

[NyaaaWhatsUpDoc]

Also, we do let clients choose whether to use wal mode or not, and if they don't use wal mode for whatever reason, the busy handler will be much more necessary.

We should probably change that honestly. Letting clients chose the mode isn't really necessary, unless there's some evidence of WAL performing worse for people. But I suspect the evidence goes in the other direction, and it limits the amount of things we have to think about when someone reports a database performance issue.

Since we have the transpile or the WASM build we can rely on WAL always being available no matter the SQLite version any distro ships.

Agreed. Also doing it now will be especially easier since we still have the "alpha" tag, where changes like this are more to be expected.

[daenney]

I took a quick look at the tests, a lot of the failures are in typeutils and the only difference seems to be the resulting url starting with http vs https. Probably just a config setting?

The rest is just "database is closed" which seems to cause goroutine blockage/runtime hangs.

[daenney]

Only thing that fails now is the envparsing test:

--- /tmp/tmp.IALNpE
+++ /tmp/tmp.JgNPAF

@@ -74,13 +74,13 @@
     "db-tls-mode": "disable",
     "db-type": "sqlite",
     "db-user": "sex-haver",
-    "dry-run": false,
+    "dry-run": true,
     "email": "",
     "host": "example.com",
     "http-client": {
-        "allow-ips": null,
-        "block-ips": null,
-        "timeout": 0,
+        "allow-ips": [],
+        "block-ips": [],
+        "timeout": 10000000000,
         "tls-insecure-skip-verify": false
     },
     "instance-deliver-to-shared-inboxes": false,

I'm guessing it needs a rebase and that should be it.

[NyaaaWhatsUpDoc]

Only thing that fails now is the envparsing test:

--- /tmp/tmp.IALNpE
+++ /tmp/tmp.JgNPAF

@@ -74,13 +74,13 @@
     "db-tls-mode": "disable",
     "db-type": "sqlite",
     "db-user": "sex-haver",
-    "dry-run": false,
+    "dry-run": true,
     "email": "",
     "host": "example.com",
     "http-client": {
-        "allow-ips": null,
-        "block-ips": null,
-        "timeout": 0,
+        "allow-ips": [],
+        "block-ips": [],
+        "timeout": 10000000000,
         "tls-insecure-skip-verify": false
     },
     "instance-deliver-to-shared-inboxes": false,

I'm guessing it needs a rebase and that should be it.

t'was actually a package init function in testrig i added messing with it. a little confusing and hard to narrow down but caught it in the end :)

[daenney]

All the tests pass omgomgomg 😄

[daenney]

Giving @tsmethurst some time to review it next week but LGTM shipit all the things.

[tsmethurst]

This mostly looks good to me, but I would still recommend we keep the retry on busy handler, as even if we at some point force admins to always use WAL mode, it's still important to implement retry on busy, as there are cases where it will be hit. From the SQLite docs:

The second advantage of WAL-mode is that writers do not block readers and readers to do not block writers. This is mostly true. But there are some obscure cases where a query against a WAL-mode database can return SQLITE_BUSY, so applications should be prepared for that happenstance.

Cases where a query against a WAL-mode database can return SQLITE_BUSY include the following:

If another database connection has the database mode open in exclusive locking mode then all queries against the database will return SQLITE_BUSY. Both Chrome and Firefox open their database files in exclusive locking mode, so attempts to read Chrome or Firefox databases while the applications are running will run into this problem, for example.

When the last connection to a particular database is closing, that connection will acquire an exclusive lock for a short time while it cleans up the WAL and shared-memory files. If a separate attempt is made to open and query the database while the first connection is still in the middle of its cleanup process, the second connection might get an SQLITE_BUSY error.

If the last connection to a database crashed, then the first new connection to open the database will start a recovery process. An exclusive lock is held during recovery. So if a third database connection tries to jump in and query while the second connection is running recovery, the third connection will get an SQLITE_BUSY error.

[ncruces]

Sorry to interject, but AFAIK, those cases are still covered by sqlite3_busy_handler or sqlite3_busy_timeout or PRAGMA busy_timeout. So, in those cases if you already have an SQLite busy timeout (or handler), you only get SQLITE_BUSY after the timeout has expired.

AFAIK, the only situation where SQLite returns SQLITE_BUSY immediately, without waiting (if you configured a busy handler), is if it detects a dead-lock. And this only happens when you upgrade a read-lock to a write-lock in the middle of a transaction, which can also be avoided if BEGIN IMMEDIATE is used for all transactions that may write. And in this case you need to do more than just retry, you need to ROLLBACK the transaction before retrying (I dunno if you're doing this).

So if you use BEGIN IMMEDIATE for write transactions, retrying after you get SQLITE_BUSY is becomes almost pointless. You can always wait a bit more, but then, why not just increase the configured timeout?

[NyaaaWhatsUpDoc]

Sorry to interject, but AFAIK, those cases are still covered by sqlite3_busy_handler or sqlite3_busy_timeout or PRAGMA busy_timeout. So, in those cases if you already have an SQLite busy timeout (or handler), you only get SQLITE_BUSY after the timeout has expired.

AFAIK, the only situation where SQLite returns SQLITE_BUSY immediately, without waiting (if you configured a busy handler), is if it detects a dead-lock. And this only happens when you upgrade a read-lock to a write-lock in the middle of a transaction, which can also be avoided if BEGIN IMMEDIATE is used for all transactions that may write. And in this case you need to do more than just retry, you need to ROLLBACK the transaction before retrying (I dunno if you're doing this).

So if you use BEGIN IMMEDIATE for write transactions, retrying after you get SQLITE_BUSY is becomes almost pointless. You can always wait a bit more, but then, why not just increase the configured timeout?

Indeed. We're currently setting _txlock=immediate so that covers the case of deadlocks under WAL possibly returning SQLITE_BUSY, (and needing to rollback the tx before another attempt).

Previously we added the retryBusy handler in our own code (as well as setting a fairly generous busy_timeout duration) as we were having issues with an older version of modernc.org/sqlite still returning quite a few SQLITE_BUSY errors. This doesn't seem to happen anymore fortunately, (I wonder if their busy handler was perhaps misbehaving previously).

[tsmethurst]

Aha OK thanks for clarifying @ncruces and @NyaaaWhatsUpDoc :)