codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.39k stars 1.9k forks source link

Bug: writing to SQLite3 db fails due to concurrency #6930

Closed dgvirtual closed 1 year ago

dgvirtual commented 1 year ago

PHP Version

7.4

CodeIgniter4 Version

4.2.1

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

SQLite3

What happened?

During the website peak usage times (when people rush to book rooms after a new period opens) the writing to SQLite3 db sometimes fails due to, I presume, concurrent attempts at writing; this is the error as logged:

ERROR - 2022-12-01 08:02:20 --> ErrorException: SQLite3::exec(): database is locked in /home/my/project/myApp/vendor/codeigniter4/framework/system/Database/SQLite3/Connection.php:129
Stack trace:
#0 [internal function]: CodeIgniter\Debug\Exceptions->errorHandler()
#1 /home/my/project/myApp/vendor/codeigniter4/framework/system/Database/SQLite3/Connection.php(129): SQLite3->exec()
#2 /home/my/project/myApp/vendor/codeigniter4/framework/system/Database/BaseConnection.php(695): CodeIgniter\Database\SQLite3\Connection->execute()
#3 /home/my/project/myApp/vendor/codeigniter4/framework/system/Database/BaseConnection.php(609): CodeIgniter\Database\BaseConnection->simpleQuery()
#4 /home/my/project/myApp/vendor/codeigniter4/framework/system/Database/BaseBuilder.php(1904): CodeIgniter\Database\BaseConnection->query()
#5 /home/my/project/myApp/vendor/codeigniter4/framework/system/Model.php(278): CodeIgniter\Database\BaseBuilder->insert()
#6 /home/my/project/myApp/vendor/codeigniter4/framework/system/BaseModel.php(738): CodeIgniter\Model->doInsert()
#7 /home/my/project/myApp/vendor/codeigniter4/framework/system/Model.php(666): CodeIgniter\BaseModel->insert()
#8 /home/my/project/myApp/app/Controllers/Booking.php(278): CodeIgniter\Model->insert()
#9 /home/my/project/myApp/vendor/codeigniter4/framework/system/CodeIgniter.php(896): App\Controllers\Booking->reserve_hour()
#10 /home/my/project/myApp/vendor/codeigniter4/framework/system/CodeIgniter.php(466): CodeIgniter\CodeIgniter->runController()
#11 /home/my/project/myApp/vendor/codeigniter4/framework/system/CodeIgniter.php(349): CodeIgniter\CodeIgniter->handleRequest()
#12 /home/my/project/public_html/myApp/index.php(55): CodeIgniter\CodeIgniter->run()
#13 {main}

Controller method Booking->reserve_hour() has this line which failed in this case: $this->bookingModel->insert($fields)

I presume the error is triggered if php does not get an immediate exclusive lock on the db file. It could be avoided by improving the Codeigniter sqlite3 driver (or wherever it would be appropriate) by using a php sqlite3 engine busyTimeout() function and setting timeout value, so that php waits a few miliseconds untill the database becomes unlocked instead of exiting and reporting error right away. However, I am not experienced enough to improve the core code myself.

Steps to Reproduce

It is not easy to reproduce during normal usage as one needs to make concurrent write requests, which requires syncronization in miliseconds.

However, exaclty such error can be simulated if one opens the database with DB Browser For SQLite, makes some change and does not save the file, and then trying an insert operation via the Codeigniter web app.

Expected Output

As the database locks only last parts of a second, I would expect the Codeigniter to wait out the time and do the insert once the sqlite lock is released by the previous process.

Anything else?

No response

kenjis commented 1 year ago

Thank you for reporting.

Do you use transactions? Or just only call $model->insert() ?

dgvirtual commented 1 year ago

@kenjis , I only use $model->insert(). in that particular place there is only one independent insert statement, so transactions would not matter, right, on a fail there is nothing to roll back?

But you got me concerned. If, in some other place, I use multiple related insert statements (like one for the main data, and others for related data, like blog entry and tags, related categories), would those insert statements be spread overt separate database locks?

kenjis commented 1 year ago

My question was not good.

Are you using transaction in somewhere in your app?

The code where the error occurred does not matter. Because another process locked the database, so the database is locked error was occurred at the code.

I don't know the lock error occurs even if transaction is not used at all.

dgvirtual commented 1 year ago

Oh, ok, so, no transactions anywhere in my code.

michalsn commented 1 year ago

This sounds more like a feature request, not a bug.

The timeout = 0 is the default value, so it's not a framework role to change it arbitrarily. Though this value should be set right after the connection is initialized, I think you should be able to set it manually in BaseController or something, and it should work.

db_connect()->simpleQuery('PRAGMA busy_timeout=2000');

There is a place for enhancements, though. I like the idea that developers may push some additional config for connection if they want to.

We can have an additional value in the connection array to set specific options.

// app/Config/Database.php
...
'port'        => 3306,
'foreignKeys' => true,
'options'     => ['PRAGMA busy_timeout=2000'],

This way, it would be a more flexible solution and could also be used by other drivers.

kenjis commented 1 year ago

I was able to reproduce the error. And PRAGMA busy_timeout=2000 solved the error.

    public function insert()
    {
        $model = model(NewsModel::class);

        // $model->db->simpleQuery('PRAGMA busy_timeout=2000');

        $title = $this->request->getServer('REQUEST_TIME_FLOAT') . ':' . microtime();
        d($model->insert([
            'title' => $title,
            'slug'  => url_title($title, '-', true),
            'body'  => $title,
        ]));
    }
$ wrk -t12 -c400 -d10s http://localhost:8000/news/insert
ERROR - 2022-12-02 11:46:52 --> ErrorException: SQLite3::exec(): database is locked in /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/Database/SQLite3/Connection.php:132
Stack trace:
#0 [internal function]: CodeIgniter\Debug\Exceptions->errorHandler(2, 'SQLite3::exec()...', '/Users/kenji/wo...', 132)
#1 /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/Database/SQLite3/Connection.php(132): SQLite3->exec('INSERT INTO `ne...')
#2 /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/Database/BaseConnection.php(678): CodeIgniter\Database\SQLite3\Connection->execute('INSERT INTO `ne...')
#3 /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/Database/BaseConnection.php(600): CodeIgniter\Database\BaseConnection->simpleQuery('INSERT INTO `ne...')
#4 /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/Database/BaseBuilder.php(2208): CodeIgniter\Database\BaseConnection->query('INSERT INTO `ne...', Array, false)
#5 /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/Model.php(329): CodeIgniter\Database\BaseBuilder->insert()
#6 /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/BaseModel.php(782): CodeIgniter\Model->doInsert(Array)
#7 /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/Model.php(718): CodeIgniter\BaseModel->insert(Array, true)
#8 /Users/kenji/work/codeigniter/ci4-news/app/Controllers/News.php(33): CodeIgniter\Model->insert(Array)
#9 /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/CodeIgniter.php(936): App\Controllers\News->insert()
#10 /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/CodeIgniter.php(501): CodeIgniter\CodeIgniter->runController(Object(App\Controllers\News))
#11 /Users/kenji/work/codeigniter/ci4-news/vendor/codeigniter4/codeigniter4/system/CodeIgniter.php(370): CodeIgniter\CodeIgniter->handleRequest(NULL, Object(Config\Cache), false)
#12 /Users/kenji/work/codeigniter/ci4-news/public/index.php(67): CodeIgniter\CodeIgniter->run()
#13 {main}
dgvirtual commented 1 year ago

@kenjis, @michalsn, thank you. I have included a code line in BaseController.php, at the very end:

db_connect()->simpleQuery('PRAGMA busy_timeout=2000');

and now the code waits for 2 seconds to complete; I have tested it the simple way: run the code with insert statement while I have the database open with edited and unsaved values in DB browser app, and then save the DB before the 2 seconds have expired. The insert operation completes successfully after a delay.

I suppose db_connect() returns the existing DB connection if there exists one, so I only need to make sure I run that statement before any write operation?

And I think it is a great idea to have busy_timeout as a db config option (either directly, or using the way @michalsn suggested.

Thanks for prompt investigation of the issue. My apps will be better now.

I am not sure about the proper procedures, but as for me I consider my issue resolved.

kenjis commented 1 year ago

I sent a PR #6939

kenjis commented 1 year ago

Closed by #6939

dgvirtual commented 1 year ago

@kenjis, I see the docs have been amended to include this definition of busyTimeout: "

busyTimeout | milliseconds (int) - Sleeps for a specified amount of time when a table is locked (SQLite3 only).

It seems to imply that with this setting enabled the php engine will pause the operation for the number of miliseconds and then try it again.

If my understanding of the text is correct, based on my observation, it is not what is happening. The paused operation is executed immediately after the write lock (busy state) is removed from the database. I have checked this is so by opening the database for editing in SQLITE browser app, changing some value to put the database into the write lock (busy) state, then starting the write operation in my Codeigniter app. The moment I hit save in SQLITE browser app, the same moment the Codeigniter app finishes the write operation. So perhaps a better description would be:

busyTimeout | milliseconds (int) - time period, before failing, over which write operation will be retried after encountering locked database (SQLite3 only).

kenjis commented 1 year ago

@dgvirtual

It came from SQLite docs:

This routine sets a busy handler that sleeps for a specified amount of time when a table is locked. The handler will sleep multiple times until at least "ms" milliseconds of sleeping have accumulated. After at least "ms" milliseconds of sleeping, the handler returns 0 which causes sqlite3_step() to return SQLITE_BUSY. https://www.sqlite.org/c3ref/busy_timeout.html

You can also sent PRs to improve the expressions.