Replace DirtyNIF execution model with a different mechanism

The dirtynif execution model is okay for most cases, but we have an issue where long running writes need to timeout / be interrupted mid execution and aborted. The only way to do that is to utilize https://sqlite.org/c3ref/progress_handler.html

I wonder if you may be able to stick with the process model as long as you assume each instance of the database is backed by one process. This way you make Elixir processes your thread pool, instead of having to deal with it in C/C++/Rust.

@josevalim

@josevalim I think that's a good idea where only one process in the supervision tree sits there waiting for another DbConnection call. This could make it easier to cancel existing running queries.

Let me noodle on this a little more.

Right, I think in this sense DBConnection may be getting more in the way than helping. Because you need serial access around SQLite3 and having a pool around serial access is not giving you anything. And when you don't need serial access (i.e. read operations), you don't need a pool at all.

DBConnection is necessary for Ecto but, even then, it should not be a requirement. There are only three features in Ecto.Adapters.SQL that require DBConnection (sandbox, stream, and disconnect_all) but they are all addressable.

Then there is the part of DBConnection as a convenience. I can think of two features it gives:

a transaction manager - if the process doing a transaction crashes, we need to abort it. but perhaps sqlite3 api itself already has conveniences for handling dangling transactions
the ownership pool

Of course there is a lot of work here, so I am not saying we should yank DBConnection, but it may be worth considering moving the DBConnection bits to ecto_sqlite3. In this project, based on the analysis above (which may be incomplete), you would only be missing the transaction manager (and it may be provided by sqlite3).

@josevalim

There might be an additional benefit to managing transactions in a sqlite specific way. Currently there's no information returned to changesets about which constraint was violated, but there seems to be some pragmas to figure those out as sqlite doesn't close transactions on errors.

What I'm wondering about @josevalim's suggestion is how using processes to serialize or not serialize access would work for transactions which only become write transactions mid execution. Wouldn't the suggested approach require knowing if a transaction does writes or not in advance?

Wouldn't having one queuing process per db instance negate WAL benefits? There still needs to be a pool of connections for concurrent reads. It's not possible to open a single connection and start multiple transactions on it. And if we try to work around that with snapshots, we'd start losing isolation.

@ruslandoga

@ruslandoga the reads can go directly to the NIF. Only the writes and operations inside a transaction would go through the process.

But how exactly would reads go directly to the NIF? Using what database connection? It can't be the one from the "writer" process since it might be in the middle of a transaction, and then we need synchronisation of reads and writes.

Ah, I think I understand now. Since the reads are "scoped" to a prepared statement, it might be possible to reuse a single database connection for multiple statements at once. I'll try it out.

UPDATE it works: ruslandoga#3; and reads seemingly became faster

iex(1)> {:ok, conn} = Exqlite.start_link(database: "demo.db", debug: [:trace])
{:ok, #PID<0.245.0>}
iex(2)> Exqlite.write(conn, "create table users(name text not null) strict")
# *DBG* <0.245.0> got call checkout from <0.252.0>
# *DBG* <0.245.0> sent ok to <0.252.0>, new state {[],<0.252.0>}
# *DBG* <0.245.0> got cast checkin
{:ok, []}
# *DBG* <0.245.0> new state {[],nil}

iex(3)> Exqlite.write(conn, "insert into users(name) values(?)", ["john"])
{:ok, []}
iex(4)> Exqlite.write(conn, "insert into users(name) values(?)", ["david"])
{:ok, []}
iex(5)> Exqlite.write(conn, "insert into users(name) values(?)", ["peter"])
{:ok, []}

iex(6)> {:ok, task_sup} = Task.Supervisor.start_link
{:ok, #PID<0.254.0>}

iex(7)> Task.Supervisor.async_stream_nolink(task_sup, 1..10000, fn _ -> Exqlite.query(conn, "select * from users") end, max_concurrency: 1000) |> Enum.into([])
[
  ok: {:ok, [["john"], ["david"], ["peter"]]},
  ok: {:ok, [["john"], ["david"], ["peter"]]},
  ok: {:ok, [["john"], ["david"], ["peter"]]},
  ok: {:ok, [["john"], ["david"], ["peter"]]},
  ok: {:ok, [["john"], ["david"], ["peter"]]}
# ...

I wonder how the statements cache would work with this approach. It'd need to be serialised somehow as well.

I think the tricky thing would be supporting concurrent reads from within transactions, which didn't yet do writes:
https://www.sqlite.org/lang_transaction.html#deferred_immediate_and_exclusive_transactions

Not sure how exqlite could know which statement within a transaction requires a SHARED LOCK (reads) vs one of the ones required for writing. Maybe it could naively check for SELECT … in the sql.

@LostKobrakai

@LostKobrakai I think all transactions would need to go through the queue. Even if a read transaction never becomes a write one, it still can't be executed without syncing with the writers first. Otherwise multiple transactions can be attempted:

BEGIN; -- from writer in process 1
INSERT INTO ...
INSERT INTO ...

                 BEGIN; -- a reader starts a transaction in process 2
                 -- SQLite complains about re-entry: "cannot start a transaction within a transaction"

COMMIT; -- writer finishes the transaction from process 1

This is actually what I was worried about in the comments above. But since reads don't have to be wrapped in transactions, single connection approach works.

It might be possible to do snapshots though, for read-only transactions without BEGIN / COMMIT, at the NIF level.

I just tried this with two table plus instances on the same database and it allowed me to start a read transaction just fine while another write transaction was started. Trying to start a write transaction I got database is locked. Though I guess that's two "connections" to sql, is it?

two table plus instances

That means two connections. I think the approach discussed in this issue is having only a single connection open to the database.

$ sqlite3 demo.db

SQLite version 3.40.1 2022-12-28 14:03:47
Enter ".help" for usage hints.
sqlite> begin;
sqlite> begin;
Runtime error: cannot start a transaction within a transaction

Given DBConnection is an abstraction over connections I'm not so sure. I'd for sure appreciate if it actually could do concurrent reads (in a transaction) given how much code in a business application requires to be run in a transaction, which usually start reading a bunch of things before they write.

I think there is danger of those transactions failing with SQLITE_BUSY so serialising all transactions seems like a safer way.

-- conn 1                                            -- conn 2
sqlite> begin;
                                                     sqlite> begin;
                                                     sqlite> select * from users;
sqlite> delete from users where name = 'patrick';
                                                     sqlite> insert into users(name) values('jacob');
                                                     Runtime error: database is locked (5)

If it's about performance, we can try coming up with some benchmarks to see at what write/read ratio a single connection approach wins / loses. My hot take is that (read)write transactions are rare and reads are common and having un-pooled reads would end up being faster on average. Better memory usage too, probably.

Call interrupt before close in disconnect to fix deadlock

Add platform condvar abstraction and cancellable busy handler

Use cancel in disconnect and NIF for set_busy_timeout

Add cancellation test suite for deadlock prevention

Call interrupt before close in disconnect to fix deadlock

Add platform condvar abstraction and cancellable busy handler

Use cancel in disconnect and NIF for set_busy_timeout

Add cancellation test suite for deadlock prevention

Call interrupt before close in disconnect to fix deadlock

Add platform condvar abstraction and cancellable busy handler

Use cancel in disconnect and NIF for set_busy_timeout

Fix deadlock when canceling queries stuck in VDBE or busy handler (#343)

I don't think I'll be able to replace the dirty nif execution model.

I agree we can't replace dirty NIFs. One option would be to still have reads go directly to dirty NIF resources. Here is a commit exploring that (vibe-coded, may be garbage, sorry):

josevalim/once-campfire-elixir@53eaefc

Transactions and writes go to a single process (either using DBConnection or doing something very similar to it) and all reads either go directly to the NIF resource (which would have a mutex) or using something like NimblePool.

The downside of this approach is discarding DBConnection and the conveniences that come with it (sandbox testing and what not).

Navigation Menu

Repository navigation

Replace DirtyNIF execution model with a different mechanism #192

52 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Replace DirtyNIF execution model with a different mechanism #192

Description

Activity

josevalim commented on Jan 19, 2023

warmwaffles commented on Jan 19, 2023

josevalim commented on Jan 20, 2023

LostKobrakai commented on Jan 24, 2023

ruslandoga commented on Feb 9, 2023

josevalim commented on Feb 9, 2023

ruslandoga commented on Feb 9, 2023

LostKobrakai commented on Feb 9, 2023

ruslandoga commented on Feb 9, 2023

LostKobrakai commented on Feb 9, 2023

ruslandoga commented on Feb 9, 2023

LostKobrakai commented on Feb 9, 2023

ruslandoga commented on Feb 9, 2023

52 remaining items

warmwaffles commented on Oct 10, 2026

josevalim commented on Oct 10, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions