Feedback on using the driver with Redka
nalgeon opened this issue · 13 comments
After trying Redka with the ncruces/go-sqlite3 driver, I've encountered some problems. Not sure if you consider them bugs or not, but this behavior is not consistent with mattn/go-sqlite3 and modernc.org/sqlite (both work fine in the cases described below).
Multiple statements in Exec
An Exec call with multiple parameterized queries results in a "sqlite3: multiple statements" error:
db, err := sql.Open("sqlite3", "data.db")
// works fine
_, err = db.Exec(`
drop table if exists t1;
drop table if exists t2;
create table t1(id integer primary key, name1 text);
create table t2(id integer primary key, name2 text);
`)
// fails with error: sqlite3: multiple statements
args := []any{"alice", "alice"}
_, err = db.Exec(`
insert into t1(name1) values(?);
insert into t2(name2) values(?);
`, args...)Full example (you have to run it locally, it does not run the playground)
Read-only mode
Running queries on a read-only DB (mode=ro connection string parameter) results in a "sqlite3: disk I/O error" error:
db1, err := sql.Open("sqlite3", "file:data.db?_mutex=no&_txlock=immediate")
db2, err := sql.Open("sqlite3", "file:data.db?_mutex=no&mode=ro")
// create the table using the first (writable) connection
_, err = db1.Exec(`
create table t1(id integer primary key, name1 text);
insert into t1(name1) values('alice');
`)
// select the data using the second (readonly) connection
// fails with error: sqlite3: disk I/O error
var name string
err = db2.QueryRow("select name1 from t1").Scan(&name)Full example (you have to run it locally, it does not run the playground)
Thanks!
The first one, I wouldn't consider a bug. I actually believe it's a misfeature of other drivers.
I'll look into their implementation, but (e.g.) I don't see how it can be correctly implemented in the face of numbered (?NNN) parameters, and especially numbered and positional (?) parameters mixed together.
The problem is that SQLite doesn't allow to bind parameters to sqlite3_exec, and sqlite3_prepare_v3 does not accept multiple statements. So, suddenly, which parameters go to which statement is up to interpretation, and IMO there is no correct answer.
To see what I mean, consider this example:
_, err = db.Exec(`
insert into t1(name1) values(?, ?3);
insert into t2(name2) values(?, ?1);
`, 1, 2, 3, 4)SQLite would see insert into t1(name1) values(?, ?3) with arguments 1, 2, 3 and 4, and bind 1 and 3.
But for the next statement, insert into t2(name2) values(?, ?1) what should happen? If this was a single statement (one big query) this fragment would get 4 and 1. But that's actually impossible to achieve, as for an isolated fragment these both get the same index (1) so they must have the same value.
And worse, at the C API level, I'm not sure I can even tell the difference between numbered and positional parameters, so I can't even complain only when you mix them (which is when there really is no right answer).
The read-only one I need to understand better. From the looks of it, it's a bug. And I haven't tested read-only connections much, so that's a great test case.
Thanks again!
Thanks for making these examples (which don't even depend on Redka!)
I couldn't reproduce the read-only one "as is" (i.e., it works for me), but I'm assuming it's because data.db was a WAL database to begin with?
With a WAL database, I can reproduce it, and it's very likely a bug with my VFS.
Should be fixed in e7f8311.
The first one, I wouldn't consider a bug. I actually believe it's a misfeature of other drivers.
I completely agree that the "multi-exec" behavior is a design choice, not a bug. Also, mattn and modernc are quite inconsistent in this regard. My example is one of the few cases where both work and produce the same result, many other combinations will cause either mattn or modernc to fail.
I'm currently using multi-execs in Redka to speed things up a bit, but I probably shouldn't.
Thanks for making these examples
Sure, anything to make a fellow maintainer's life a little easier. Still, I managed to miss the WAL part :)
I'm assuming it's because data.db was a WAL database to begin with
Yep. Sorry, I missed that in the example.
Should be fixed
Thank you! It works now 🎉
I'll try converting multi-execs to single-execs in Redka, re-run the tests with the ncruces driver, and let you know if anything new pops up.
Maybe I'm missing something here, but it seems like the shared cache for memory mode doesn't work properly. If the first connection creates a table, the second connection does not see it ("sqlite3: SQL logic error: no such table: t1").
db1, err := sql.Open("sqlite3", "file:data.db?mode=memory&cache=shared")
db2, err := sql.Open("sqlite3", "file:data.db?mode=memory&cache=shared")
// create the table using the first connection
_, err = db1.Exec(`
drop table if exists t1;
create table t1(id integer primary key, name1 text);
insert into t1(name1) values('alice');
`)
// select the data using the second connection
// fails with error: sqlite3: SQL logic error: no such table: t1
var name string
err = db2.QueryRow("select name1 from t1").Scan(&name)The same code works fine with mattn/modernc drivers.
Some benchmarks comparing mattn, modernc and ncruces drivers for set (two inserts, single write connection) and get (select joining two tables, 8 concurrent read connections) operations on MacBook Air M1:
mattn
SET: 26954.18 requests per second, p50=0.215 msec
GET: 104602.52 requests per second, p50=0.063 msec
modernc
SET: 20889.91 requests per second, p50=0.279 msec
GET: 83892.62 requests per second, p50=0.087 msec
ncruces
SET: 13232.76 requests per second, p50=0.431 msec
GET: 67385.45 requests per second, p50=0.119 msec
Pragmas used:
pragma journal_mode = wal;
pragma synchronous = normal;
pragma temp_store = memory;
pragma mmap_size = 268435456;
pragma foreign_keys = on;
Connection strings:
read-write db:
file:data.db?_mutex=no&_pragma=temp_store%3Dmemory&_pragma=mmap_size%3D268435456&_pragma=foreign_keys%3Don&_pragma=journal_mode%3Dwal&_pragma=synchronous%3Dnormal&_txlock=immediate
read-only db:
file:data.db?_mutex=no&_pragma=synchronous%3Dnormal&_pragma=temp_store%3Dmemory&_pragma=mmap_size%3D268435456&_pragma=foreign_keys%3Don&_pragma=journal_mode%3Dwal&mode=ro
Unfortunately, I do not support shared cache (its use is discouraged), and even if I did, I would not be able to support it for the purpose of sharing memory databases across connections, due to architecture constraints (for this driver, each connection is completely isolated from the others, like independent OS processes).
The supported alternative (and increasingly recommended by SQLite developers), is the memdb VFS (which I believe is supported by all drivers). I'm sure modernc has it, and mattn also does if compiled with SQLITE_ENABLE_DESERIALIZE, which became the default in 2021.
db1, err := sql.Open("sqlite3", "file:/data.db?vfs=memdb&_txlock=immediate"")
db2, err := sql.Open("sqlite3", "file:/data.db?vfs=memdb&mode=ro")The slash in the name is important, as that makes it a shared database.
If you can share the benchmark code, I'll see if there's anything that can be improved.
I don't currently support mmap_size, as the way it's implemented in SQLite is… IDK.
But having a good benchmark that shows its advantages would let me explore alternative implementation strategies.
Edit: I suspect than might not be the biggest issue. My locking might not be as efficient. The first thing I'd try on a mac would be to rerun the benchmark with the sqlite3_flock build tag.
One of the ways I got good results in this benchmark was to improve scanning of queries with many result columns, which admittedly is not the sweet spot for Redka.
The supported alternative (and increasingly recommended by SQLite developers), is the memdb VFS
Thank you, this is very helpful! I've changed all the mode=memory&cache=shared code to vfs=memdb, and it works like a charm.
I've also tested Redka with the ncruces driver, and it passes all the tests. So I've added your driver to the list of supported ones in the docs.
If you can share the benchmark code, I'll see if there's anything that can be improved.
I use the Redis benchmarking tool redis-benchmark against the Redka API as described here. I'm afraid it won't be of much use to you.
Thank you very much for your help, I'm happy to see Redka working with ncruces/go-sqlite3! 🥳
That's great! I'll still look at the multiple statements, to see if I can make sense of the other drivers' take on this.
At a minimum I'll open a few discussions on these incompatibilities.
Thanks @nalgeon!
I released v0.16.2 with the readonly bug fix.
There's some performance gain (~10% in some benchmarks) to be had with the sqlite3_flock BSD WAL implementation. Unfortunately that's not as compliant with SQLite locking protocol (e.g. it can't be used with Litestream). I may try to get best of both worlds, but it complicates locking somewhat (and likely doesn't get the full 10%).
I'll profile your benchmark, time permitting.
Also see:
Great news, thank you Nuno!