Mtasa-blue: Some databases are inaccessible since r20477

[ArranTuna]

Describe the bug
r20468 works fine the next up r20477 does not, there is a huge debug spam when launching the server:

dbExec failed; (5) database is locked
and
dbPoll failed; database is locked

So this commit obviously caused this: https://github.com/multitheftauto/mtasa-blue/commit/6c3be117a7c5f4a25462a4c89f74540376c19a62

To reproduce

1. Run a server with r20477 or above and do database stuff and get the spam.
2. The server is Windows with 64 bit build.

srun con = dbConnect("sqlite", ":/registry.db")
srun dbExec(con, "CREATE TABLE IF NOT EXISTS bob (bobs TEXT)")

It will return debug warning "database is locked"

Expected behaviour

Screenshots

Version
Windows 64 bit r20477+

Additional context

[ccw808]

I think we should revert 6c3be11

[qaisjp]

Yes we should revert it

[qaisjp]

/cc @pentaflops

[pentaflops]

Show your sql queries

[ArranTuna]

Can be reproduced this easy:

srun con = dbConnect("sqlite", ":/registry.db")
srun dbExec(con, "CREATE TABLE IF NOT EXISTS bob (bobs TEXT)")

It will return "database is locked"

[sbx320]

Unable to reproduce on master or r20546 on x64 or x86 builds with

run con = dbConnect("sqlite", ":/registry.db")
run dbExec(con, "CREATE TABLE IF NOT EXISTS bob (bobs TEXT)")

[ArranTuna]

I have just tested now and was unable to reproduce it until I started all the other resources. Now that I started them all another script that uses dbcon = dbConnect("sqlite", ":/registry.db") is returning database is locked so it must after a certain amount of resources have connected to it (it must be more than 2 as I tested just 2) it becomes broken so I will try to see how many it needs before it breaks.

[ArranTuna]

I've found a way to reproduce it on default resources with one runcode line:

run con = dbConnect("sqlite", ":/registry.db") dbExec(con, "CREATE TABLE IF NOT EXISTS bob10 (bobs TEXT)") con2 = dbConnect("sqlite", ":/registry.db") dbExec(con2, "CREATE TABLE IF NOT EXISTS bob11 (bobs TEXT)")

Strangely, it works fine if the tables are already created but when calling both of these at the same time with fresh table names, I got the database is locked warning.

[sbx320]

Thanks, that works for reproduction.

SQLite internally saves the schema as plain text. As a result creating new tables, requires a global lock on the database. Currently we don't wait at all for the database to unlock, so concurrent queries break it. That's easy to fix by setting a busy timeout to wait a couple milliseconds for the database to be ready again. However this causes another issue where I'm not quite sure about.

The issue relates to schema modifications from within two concurrent transactions. Interestingly I can reproduce this even outside of MTA using the following procedure:

• Open two sqlite3 instances on the same database
• Instance 1: BEGIN TRANSACTION
• Instance 1: CREATE TABLE IF NOT EXISTS bob10 (bobs TEXT)
• Instance 2: BEGIN TRANSACTION
• Instance 2: CREATE TABLE IF NOT EXISTS bob11 (bobs TEXT)
• Instance 2: END TRANSACTION - This will succeed and create the table
• Instance 1: END TRANSACTION - This will fail with a Disk I/O Error

Not quite sure if this is a SQLite bug or intended behavior. A fix for us could be to always share sqlite database handles refering to the same database file.

[ccw808]

Another solution would be to disable concurrent transactions for SQLite

[Dutchman101]

Strangely, it works fine if the tables are already created but when calling both of these at the same time with fresh table names, I got the database is locked warning.

That's not exclusively the case.. i saw this issue with a script saving killstats of thousands of players, obviously the sqlite DB is full with data (nothing was reset) and randomly, on restarting the server, it started locking up and failing to read the DB (causing script malfunction).

[gameplay]\killstats\server.lua:9: poll failed; database is locked
(local result = query:poll(-1))

Relevant part of the script in question:

local SQL = Connection("sqlite", "kills.db")
SQL:exec("CREATE TABLE IF NOT EXISTS player_kills (account_name varchar(255), account_kills INT)")

local function getDatabaseAccountData(accName)
    if accName then
        local query = SQL:query("SELECT * FROM player_kills WHERE account_name=?", accName)
        local result = query:poll(-1)

        if result and type(result) == "table" then
            if (#result) > 0 then
                return result[1]["account_kills"]
            end
        end
    end
    return false
end

So yeah, server nightlies are really unstable with this bug.. sqlite databases can randomly fail.

[ArranTuna]

That's not exclusively the case..

This is just the only way I could reproduce it with a runcode line. The bug originally occurred when starting the server (and happened each time) even though the act of starting the server wasn't creating any new tables.

[AlexTMjugador]

Another solution would be to disable concurrent transactions for SQLite

Even when using the rollback journal, that would also mean that concurrent read-only transactions would perform worse, as SQLite is clever about not exclusively locking the entire database file when the only concurrent operations are reads. Moreover, if WAL is used, the performance penalty is even bigger because, as the SQLite documentation states, "readers do not block writers and a writer does not block readers. Reading and writing can proceed concurrently."

Not quite sure if this is a SQLite bug or intended behavior. A fix for us could be to always share sqlite database handles refering to the same database file.

This random StackOverflow answer states that "commands like CREATE TABLE work perfectly well inside transactions", so maybe it's related with the isolation level or how SQLite is used?

[ccw808]

Another solution would be to disable concurrent transactions for SQLite

What I meant was disable 6c3be11 for SQLite

If that can't be done easily then revert 6c3be11

[ccw808]

Please someone revert 6c3be11