tursodatabase / libsql-client-ts

TypeScript/JavaScript client API for libSQL
https://docs.turso.tech/sdk/ts/quickstart
MIT License
226 stars 32 forks source link

Make local interactive transactions work (better-sqlite3 is fully blocks in its transaction API) #6

Closed CodingDoug closed 1 year ago

CodingDoug commented 1 year ago

We have future plans to support interactive transactions using websockets, and the client should offer the same API using a local database. The problem is that the public API we need for that is fundamentally incompatible with the way better-sqlite3 works. From its documentation:

Easy-to-use synchronous API (better concurrency than an asynchronous API... yes, you read that correctly)

When a transaction is started, it requires the callback code to run synchronously on the main thread that kicked it off. As such, we cannot simply port it to our expected API, which needs to be async to support websocket interactions.

honzasp commented 1 year ago

Yes, we won't be able to implement better-sqlite3 at all (not just interactive transactions!), because that API is synchronous.

CodingDoug commented 1 year ago

@honzasp it actually works OK for individual statements. We're at least able to return a promise that the caller can use to get the results. It might actually block the caller as an implementation detail, but the API doesn't suffer.

honzasp commented 1 year ago

If existing code that uses better-sqlite3 expects to receive a result synchronously, it will break if we return a promise instead.

Also, in JavaScript, if we wanted to block the main thread (which is something that we should almost never do in serious applications), we would have to spawn a WebWorker to do the IO.

glommer commented 1 year ago

does the other package (node-sqlite3) has the same issue ?

glommer commented 1 year ago

node-sqlite3 claims: "Asynchronous, non-blocking SQLite3 bindings for Node.js."

CodingDoug commented 1 year ago

It shouldn't. They use callbacks for everything, and have constructs for expressing statements in serial (blocking) and parallel. The problem is transactions. Looks like it requires a bunch of special care: https://stackoverflow.com/questions/53299322/transactions-in-node-sqlite3

Even the solution there doesn't really show interactive transactions (what they are doing is just a batch wrapped in begin/commit with the result coming in a single promise). We'd have to go through extra lengths to make sure that a single db connection doesn't try to do more than the single transaction.

honzasp commented 1 year ago
honzasp commented 1 year ago

and have constructs for expressing statements in serial (blocking) and parallel.

In node-sqlite3, .serialize() does not make the operations blocking, it makes them serial: they are still executed in the background, the library just does not execute the operations in parallel.

For example, the following code:

const sqlite3 = require("sqlite3");
const db = new sqlite3.Database(":memory:");
db.serialize(() => {
    console.log("execute 1");
    db.get("SELECT 1", (err) => {
        if (err) throw err;
        console.log("callback 1");
    });

    console.log("execute 2");
    db.get("SELECT 2", (err) => {
        if (err) throw err;
        console.log("callback 2");
    });
});
db.close();

prints

execute 1
execute 2
callback 1
callback 2

because the callbacks are called asynchronously. If we didn't use .serialize(), the only difference would be that callback 2 might get printed before callback 1.

CodingDoug commented 1 year ago

@honzasp Do we have a plan to address this for seamless local/remote switching, or are we accepting that local development is not really an option for this library?

honzasp commented 1 year ago

If we are talking about using better-sqlite3 for file:// URLs in our library, then it shouldn't be a problem: individual SQLite statements should in most cases be executed so quickly that blocking the main thread isn't a big issue. better-sqlite3 claims that it is in most cases much faster to block the main thread for a short while than delegating to a thread pool.

For this reason, #10 keeps using the blocking better-sqlite3. If it turns out to be a problem in the future, it should be fairly straightforward to switch to sqlite3, which uses a thread pool and is nonblocking.

honzasp commented 1 year ago

Local interactive transactions should work as expected.