prisma / prisma

Next-generation ORM for Node.js & TypeScript | PostgreSQL, MySQL, MariaDB, SQL Server, SQLite, MongoDB and CockroachDB
https://www.prisma.io
Apache License 2.0
38.81k stars 1.52k forks source link

Support setting a timeout for SQLite #2955

Closed internalfx closed 2 years ago

internalfx commented 4 years ago

Problem

SQLite queries fail immediately if the DB is locked.

I blindly tried passing arguments similarly to how the docs show for PostgreSQL, needless to say, it didn't work.

datasource db {
  provider = "sqlite"
  url      = "file:../db.sqlite?timeout=5000"
}
pantharshit00 commented 4 years ago

@internalfx Try using the socket_timeout parameter.

Ref: https://docs.rs/quaint/0.2.0-alpha.12/quaint/connector/struct.SqliteParams.html#structfield.socket_timeout

https://github.com/prisma/quaint/blob/52e93922e9231fa58c1ed747df9c385c23fd1b3d/src/connector/sqlite.rs#L31

internalfx commented 4 years ago

Thanks for the tip @pantharshit00!

However, it's not having any effect.

here is my current datasource.

datasource sqlite {
  provider = "sqlite"
  url      = "file:../db.sqlite?socket_timeout=5000"
}

I've tried many different numbers 5, 5000, 9999, 9999999

Nothing has any effect.

pimeys commented 4 years ago

Hey @internalfx, SQLite is a bit interesting in databases due to it staying in the same memory as the application you're running. We do not have socket communication with the system; it's more of a Mutex<Database>.

I think our defaults for SQLite are wrong. What I would do is the following:

Socket timeout does nothing for SQLite due to us not using a socket with it. If one connection starts to be a bottleneck, I'd consider databases with more advanced locking mechanisms, such as PostgreSQL or MySQL.

internalfx commented 4 years ago

Thanks for the help @pimeys however there still seems to be no difference.

I used the following parameters...

file:../db.sqlite?connection_limit=1&connect_timeout=30

When that had no effect I also tried 30000 just in case it was milliseconds.

I opened the DB in another program and started a transaction (locking the DB) to test, prisma still throws an error immediately when attempting a write.

PrismaClientUnknownRequestError: Error occurred during query execution:
ConnectorError(ConnectorError { user_facing_error: None, kind: ConnectionError(Timeout("SQLite database is busy")

Any ideas?

pimeys commented 4 years ago

SQLite is not really meant for multi-user use cases. It's much better if you just have one writer to the database. Every connection locks the whole thing while reading or writing, so you only want one of them to that database.

That error comes directly from the database. We really can't do anything to it, if SQLite decides to error out while holding multiple connections into it.

If you need more connections, I'd consider trying out PostgreSQL.

internalfx commented 4 years ago

@pimeys thank you for taking the time to respond.

SQLite is not really meant for multi-user use cases. It's much better if you just have one writer to the database.

Agreed. But let me preface our future discussion by saying I'm not just using SQLite for the fun of it. I've used it in the past and have studied it's capabilities and limitations.

Every connection locks the whole thing while reading or writing, so you only want one of them to that database.

That is not entirely correct, SQLite can handle multiple readers quite well.

That error comes directly from the database. We really can't do anything to it, if SQLite decides to error out while holding multiple connections into it.

I think you might be mistaken here. SQLite directly supports a "busy timeout". See docs

Also the sqlite3 npm package supports the timeout as well. See docs

I can only assume that prisma could do something similar.

If you need more connections, I'd consider trying out PostgreSQL.

I work with small companies, usually developing in-house software to be used by at most 5-10 people. SQLite is more than capable of keeping up. And in "production" prisma is the only process that will be communicating with the DB in the vast majority of all cases.

However, if I need to do any maintenance of any kind. I can cause the DB to start throwing BUSY errors. It's not a problem for the DB to just wait a few seconds. Hence, why I am trying to set a timeout.

The program having to wait is very acceptable in this case, but the busy errors are troublesome.

Is there any other way I can set the timeout?

pimeys commented 4 years ago

AH! Now I see what you want to do!

Yeah, busy timeout would be quite easy to set to the database. We'll be candidating this to 2.4.0.

janpio commented 4 years ago

I assume our "busy timeout" is 0 then right now @pimeys and we might want to change that or at least open up to a parameter?

pimeys commented 4 years ago

We should have a parameter and document it. Should we go with socket_timeout=N or busy_timeout=N? The former would be in line with the other drivers, latter would reflect better what the value actually is...

pimeys commented 4 years ago

This is just a pragma for the connection, so the work here is mostly deciding the naming.

internalfx commented 4 years ago

I assume our "busy timeout" is 0 then right now

@janpio based on my testing that seems to be the case

@pimeys , @janpio As always thanks for taking time out of your busy day to help with this.

internalfx commented 4 years ago

@pimeys personally, I tend to prefer busy_timeout=N. I like features that are so obvious they don't need documentation.

pimeys commented 4 years ago

Ok, I had finally some time to look into this issue. Reading our code it seems we do already support busy timeouts, this is what I found:

if let Some(timeout) = params.socket_timeout {
    conn.busy_timeout(timeout)?;
};

Meaning, as with other connectors the SQLite connector has the socket_timeout parameter, that does set the busy_timeout in SQLite in seconds. I needed to do a test with it, so I fired our SQLite datamodel and set the connection string into: file:./db/Chinook.db?socket_timeout=10&connection_limit=1 and tried to fire some queries.

First query, finding all users succeeded as one would guess. After that I connected to the database with sqlite3 client, manually locking the database:

PRAGMA locking_mode = EXCLUSIVE;
BEGIN EXCLUSIVE;

Now when querying the database with Prisma, the request will just wait for 10 seconds, after which a Timeout error is raised. Triggering another query and exiting the sqlite3 client in another terminal before hitting the timeout released the lock for the client, which got results successfully.

Is there anything else I should test, otherwise I'm closing the issue?

janpio commented 4 years ago

So https://github.com/prisma/prisma/issues/2955#issuecomment-656384094 should have worked? Is this parameter documented as doing what it does?

pimeys commented 4 years ago

For some reason that didn't work for @internalfx. We've always had this parameter for SQLite though and it's documented here https://docs.rs/quaint/0.2.0-alpha.12/quaint/pooled/index.html#sqlite

Don't know if our JS docs has this documented though...

internalfx commented 4 years ago

@pimeys I will do another test today with an equivalent connection string to what you posted above and get back to you.

Perhaps there is an issue with parsing the arguments from the prisma schema? There is another bug (that I have not created an issue for yet, sorry) where calling prisma migrate with these SQLite arguments causes an error.

internalfx commented 4 years ago

@pimeys OK...this is very strange....

So I modified my prisma.schema to match the same connection string example you gave above...

datasource db {
  provider = "sqlite"
  url      = "file:../aam.sqlite?socket_timeout=10&connection_limit=1"
}

Then I started a transaction....

BEGIN EXCLUSIVE;

Then I tried to update a record with prisma and had instant failure...

Error occurred during query execution:\n' +
    'ConnectorError(ConnectorError { user_facing_error: None, kind: ConnectionError(Timeout("SQLite database is busy")\n'

Any idea what I'm doing wrong?

pimeys commented 4 years ago

One thing we need to be sure of now is could it be our build pipeline is using some ancient version of SQLite that is baked into Prisma. Can we see some versions you have @internalfx?

prisma --version

Just to be sure where we are client-wise, and then with prisma, query with the following statement with the raw interface:

SELECT sqlite_version() AS version

I get version 3.31.0 as an answer on Ubuntu 20.04 and the same answer on Arch Linux. What we SHOULD do is we should pin a certain SQLite version source code when we build Prisma. There is a possibility of using the system version of SQLite, and in some systems that might be too old to have the busy timeout enabled. Might be far-fetched, but this is something we should at least be sure of before we continue our debugging.

Also it would help if you'd listed your system info here. Different build pipelines behave differently.

internalfx commented 4 years ago

$ npx prisma --version

@prisma/cli          : 2.3.0
Current platform     : debian-openssl-1.1.x
Query Engine         : query-engine e11114fa1ea826f9e7b4fa1ced34e78892fe8e0e (at node_modules/@prisma/cli/query-engine-debian-openssl-1.1.x)
Migration Engine     : migration-engine-cli e11114fa1ea826f9e7b4fa1ced34e78892fe8e0e (at node_modules/@prisma/cli/migration-engine-debian-openssl-1.1.x)
Introspection Engine : introspection-core e11114fa1ea826f9e7b4fa1ced34e78892fe8e0e (at node_modules/@prisma/cli/introspection-engine-debian-openssl-1.1.x)
Format Binary        : prisma-fmt e11114fa1ea826f9e7b4fa1ced34e78892fe8e0e (at node_modules/@prisma/cli/prisma-fmt-debian-openssl-1.1.x)
Preview Features     : transactionApi
const result = await prisma.queryRaw(`SELECT sqlite_version() AS version;`)
console.log(result)

// [ { version: '3.31.0' } ]
$ sqlite3 --version
3.31.1 2020-01-27 19:55:54 3bfa9cc97da10598521b342961df8f5f68c7388fa117345eeb516eaa837balt1
internalfx commented 4 years ago

Also I'm running Pop!_OS. Which is based on Ubuntu 20.04.

pimeys commented 4 years ago

@pantharshit00 Could you do a quick test and confirm if it works or not?

pimeys commented 4 years ago

Ah, he's on vacation until 3rd of August...

pimeys commented 4 years ago

HA! It is a bug! We have some weirdo param filtering here, and it never gets the timeout parameter.

For some reason my build has a default timeout of five seconds, and I was testing with that value, so I didn't notice it never worked. When I used a different value I still got a timeout error after five seconds. I'll fix this now...

internalfx commented 4 years ago

:tada: Awesome @pimeys !

And I want to say thank you again for all the work you've put into this issue.

pimeys commented 4 years ago

Sorry for slow response. We have quite a full schedule of things to do...

pimeys commented 4 years ago

Oh, the Quaint issue closed this a bit early. Engine tests are running now, so the fixed build should be available later tonight as a dev build, or if patient, in 2.4.0.

internalfx commented 4 years ago

Sounds good. I'll probably wait for 2.4.0.

internalfx commented 4 years ago

Just upgraded to 2.4.0. And unfortunately I'm still getting instant timeouts.

schema.prisma

datasource db {
  provider = "sqlite"
  url      = "file:../aam.sqlite?socket_timeout=30&connection_limit=1"
}

ConnectorError(ConnectorError { user_facing_error: None, kind: ConnectionError(Operation timed out (SQLite database is busy)) })

@prisma/cli          : 2.4.0
Current platform     : debian-openssl-1.1.x
Query Engine         : query-engine 6c777331554df4c3e0a90dd841339c7b0619d0e1 (at node_modules/@prisma/cli/query-engine-debian-openssl-1.1.x)
Migration Engine     : migration-engine-cli 6c777331554df4c3e0a90dd841339c7b0619d0e1 (at node_modules/@prisma/cli/migration-engine-debian-openssl-1.1.x)
Introspection Engine : introspection-core 6c777331554df4c3e0a90dd841339c7b0619d0e1 (at node_modules/@prisma/cli/introspection-engine-debian-openssl-1.1.x)
Format Binary        : prisma-fmt 6c777331554df4c3e0a90dd841339c7b0619d0e1 (at node_modules/@prisma/cli/prisma-fmt-debian-openssl-1.1.x)
Preview Features     : transactionApi

@pimeys Any Ideas?

internalfx commented 4 years ago

It seems as if somehow the parameters still fail to reach the DB engine.

@pantharshit00 Can we reopen this issue?

internalfx commented 4 years ago

I would like to propose that a default timeout be set in the driver.

  1. A default timeout should not cause any problems for current users
  2. It's a very useful workaround until this issue can be solved

Thoughts?

internalfx commented 4 years ago

Sometimes I hit this issue using only prisma's tools in development.

For example running prisma studio while also working on a project. Even a short timeout would make 99% of these problems go away and I believe should be a default.

pantharshit00 commented 4 years ago

Hey @internalfx Julius is on a holiday right now 😅

I can confirm that we are not picking up the timeout even after the work that Julius did. I will pass this ticket to someone else who has worked on that of the codebase. My inital findings suggests that even though we are parsing it correctly now, we are not passing it to our db lib quaint.

cc / @mavilein @tomhoule

tomhoule commented 4 years ago

So I just checked, and the param seems to be passed fine (we pass the whole connection url to quaint, quaint parses it and extracts it). It is possible that quaint was not upgraded before 2.4.0, but it should be now. Can you try again with the latest dev release (@prisma/cli@2.5.0-dev.36 as of right now)?

internalfx commented 4 years ago

@tomhoule I will do that and get back to you

internalfx commented 4 years ago

Unfortunately, I still can't get it to work.

OK. Here are the steps I've taken....

I upgraded prisma cli nvm exec yarn add @prisma/cli@2.5.0-dev.36

Removed prisma client nvm exec yarn remove @prisma/client

Regenerated prisma client

info Direct dependencies
└─ @prisma/client@2.5.0-dev.36
info All dependencies
└─ @prisma/client@2.5.0-dev.36
Done in 5.66s.

✔ Installed the @prisma/client and prisma packages in your project

✔ Generated Prisma Client to ./node_modules/@prisma/client in 72ms

You can now start using Prisma Client in your code:

import { PrismaClient } from '@prisma/client'
// or const { PrismaClient } = require('@prisma/client')

const prisma = new PrismaClient()

Ran my program after starting a transaction in another program.. nvm run app.js

Busy error shows up immediately

PrismaClientUnknownRequestError: Error occurred during query execution:
ConnectorError(ConnectorError { user_facing_error: None, kind: ConnectionError(Operation timed out (SQLite database is busy)) })
    at PrismaClientFetcher.request (/home/bmorris/internalfx/projects/centric-server/node_modules/@prisma/client/src/runtime/getPrismaClient.ts:1225:15)

I should note that I use better-sqlite in my project as well and it is able to use a timeout.

Is there any information I can provide that would be helpful?

internalfx commented 4 years ago

I also looked to see what newer versions are available.

I just tried the above process again with @prisma/client@2.5.0-dev.43

No luck.

janpio commented 4 years ago

Is there any information I can provide that would be helpful?

Nope, you done all you can - as @pantharshit00 confirmed the problem again we will need to look into the code and figure out what is going wrong here. On our list now, so hopefully someone will get to it soon (again).

pimeys commented 4 years ago

I'm trying this with the current Prisma master using the connection string:

file:./db/Chinook.db?connection_limit=1&socket_timeout=20

The client waits 20 seconds and then gives Timeout. Tested it also with 2.5.0-dev.43 and it works the same. Can we see your connection string here to be sure you have the right parameters?

pantharshit00 commented 4 years ago

I can't also reproduce this in the latest dev version @internalfx

I have put this example together which you can use to test this out: https://github.com/harshit-test-org/prisma-sqlite-timeout-demo

internalfx commented 4 years ago

@pimeys @pantharshit00 Thanks for posting a repo! great idea.

Ok, this definitely cleared up some confusion between both parties. I should have been more clear, I've never had a problem with reads.

I have WAL mode set on the DB. which allows readers and writers to not block each other.

The only time I've ever had a problem with prisma not waiting properly, is on writes.

@pantharshit00 I'm going to send a PR to your repo with the changes I tested with to reproduce the issue.

pimeys commented 4 years ago

I just tested with a simple insert. We wait 20 seconds (which is the set timeout) and get the timeout error after that.

internalfx commented 4 years ago

@pimeys WHAT?!

Can you test with the code I just sent @pantharshit00 ?

https://github.com/harshit-test-org/prisma-sqlite-timeout-demo/pull/1

pimeys commented 4 years ago
sqlite> pragma journal_mode;
journal_mode
------------
wal
sqlite> PRAGMA locking_mode = EXCLUSIVE;
locking_mode
------------
exclusive
sqlite> BEGIN EXCLUSIVE;

Running an upsert to the database with another connection from Prisma. Waits 20 seconds, throws a timeout.

internalfx commented 4 years ago

ok.....now this is getting weird....

I've updated my repo, with two test cases. There are some new clues.

https://github.com/internalfx/prisma-sqlite-timeout-demo

I've also posted a video showing the problem...

https://github.com/internalfx/prisma-sqlite-timeout-demo/raw/master/prisma-bug.mkv

pimeys commented 4 years ago

It's getting quite late, but I watched the video and was able to find out what's the difference between the GUI app and the database. If you set pragma locking_mode to exclusive, the busy timeout works and all good. If the locking_mode is set to normal, one can begin exclusive and Prisma will error with Timeout immediately.

Weird...

internalfx commented 4 years ago

I know right?!?

Thanks for having a look.

I'll see if I can dig up any useful information.

pimeys commented 4 years ago

Please dig a bit more what the C++ wrapper in better-sqlite3 actually does. I can't really find anything different from what we do from their codebase, but I might miss something due to their codebase being full of macros...

internalfx commented 4 years ago

@pimeys Is this relevant?

https://github.com/rusqlite/rusqlite/issues/767

pimeys commented 4 years ago

Updated to version 0.24.0 and still something rewrites the busy handler in normal locking mode. Maybe as a workaround would it be possible to set the GUI tool to make an exclusive lock before editing?

internalfx commented 4 years ago

It's possible, but then it can't edit the DB while the program is running.

This is not an emergency, BTW. If it's late get some sleep.

And thanks for taking the time to look into this, at least we've isolated the real issue now.