Skip to content

Commit 2224d1a

Browse files
JanJakesJanJakes
authored
Improve concurrent database access (#361)
## Summary Reduces lock contention under concurrent access by avoiding unnecessary write locks for read-only queries. 1. **Skip the wrapper transaction for SELECT statements.** A standalone SELECT translates to a single SQLite query, so the implicit autocommit transaction provides sufficient consistency — no explicit `BEGIN`/`COMMIT` needed. 2. **Use deferred `BEGIN` for other read-only wrapper transactions.** SHOW and DESCRIBE queries still use a wrapper transaction (they may translate to multiple SQLite queries), but now use a deferred `BEGIN` (SHARED lock) instead of `BEGIN IMMEDIATE` (RESERVED lock). ### Background The wrapper transaction previously used `BEGIN IMMEDIATE` for all queries, including read-only SELECTs. This acquired a RESERVED (write) lock on every query — but SQLite only allows one RESERVED lock at a time. Under concurrent load (e.g., multiple PHP-FPM workers handling requests simultaneously), all processes competed for the single write lock. When the 10-second busy timeout was exceeded, SQLite returned `SQLITE_BUSY` ("database is locked", error 5). SHARED locks, acquired by a deferred `BEGIN`, are compatible with RESERVED locks — multiple readers can proceed concurrently alongside an active writer. Fixes #318
1 parent 1e4a19a commit 2224d1a

2 files changed

Lines changed: 158 additions & 3 deletions

File tree

‎packages/mysql-on-sqlite/src/sqlite/class-wp-pdo-mysql-on-sqlite.php‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -883,12 +883,34 @@ public function query( string $query, ?int $fetch_mode = null, ...$fetch_mode_ar
883883
null === $child_node
884884
|| 'beginWork' === $child_node->rule_name
885885
|| $child_node->has_child_node( 'transactionOrLockingStatement' )
886+
|| $child_node->has_child_node( 'selectStatement' )
886887
) {
887888
$wrap_in_transaction = false;
888889
} else {
889890
$wrap_in_transaction = true;
890891
}
891892

893+
/*
894+
* Detect read-only statements before opening the wrapper transaction.
895+
*
896+
* [GRAMMAR]
897+
* simpleStatement: selectStatement | showStatement | utilityStatement | ...
898+
*/
899+
if ( null !== $child_node && $child_node->has_child_node() ) {
900+
$statement_node = $child_node->get_first_child_node();
901+
if (
902+
'selectStatement' === $statement_node->rule_name
903+
|| 'showStatement' === $statement_node->rule_name
904+
) {
905+
$this->is_readonly = true;
906+
} elseif ( 'utilityStatement' === $statement_node->rule_name ) {
907+
$utility_subnode = $statement_node->get_first_child_node();
908+
if ( null !== $utility_subnode && 'describeStatement' === $utility_subnode->rule_name ) {
909+
$this->is_readonly = true;
910+
}
911+
}
912+
}
913+
892914
if ( $wrap_in_transaction ) {
893915
$this->begin_wrapper_transaction();
894916
}
@@ -1366,7 +1388,6 @@ private function execute_mysql_query( WP_Parser_Node $node ): void {
13661388
$this->execute_transaction_or_locking_statement( $node );
13671389
break;
13681390
case 'selectStatement':
1369-
$this->is_readonly = true;
13701391
$this->execute_select_statement( $node );
13711392
break;
13721393
case 'insertStatement':
@@ -1444,14 +1465,12 @@ private function execute_mysql_query( WP_Parser_Node $node ): void {
14441465
$this->execute_set_statement( $node );
14451466
break;
14461467
case 'showStatement':
1447-
$this->is_readonly = true;
14481468
$this->execute_show_statement( $node );
14491469
break;
14501470
case 'utilityStatement':
14511471
$subtree = $node->get_first_child_node();
14521472
switch ( $subtree->rule_name ) {
14531473
case 'describeStatement':
1454-
$this->is_readonly = true;
14551474
$this->execute_describe_statement( $subtree );
14561475
break;
14571476
case 'useCommand':

‎packages/mysql-on-sqlite/tests/WP_SQLite_Driver_Concurrency_Tests.php‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
<?php
2+
3+
use PHPUnit\Framework\TestCase;
4+
5+
/**
6+
* Tests for concurrent access to the same SQLite database file.
7+
*/
8+
class WP_SQLite_Driver_Concurrency_Tests extends TestCase {
9+
/**
10+
* Path to the temporary SQLite database file used in file-based tests.
11+
*
12+
* @var string|null
13+
*/
14+
private $db_path;
15+
16+
public function setUp(): void {
17+
$this->db_path = tempnam( sys_get_temp_dir(), 'wp_sqlite_' );
18+
unlink( $this->db_path ); // Remove so SQLite creates a fresh database.
19+
}
20+
21+
public function tearDown(): void {
22+
foreach ( array(
23+
$this->db_path,
24+
$this->db_path . '-wal',
25+
$this->db_path . '-shm',
26+
$this->db_path . '-journal',
27+
) as $path ) {
28+
if ( is_string( $path ) && file_exists( $path ) ) {
29+
unlink( $path );
30+
}
31+
}
32+
$this->db_path = null;
33+
}
34+
35+
public function testSelectQueryIsNotWrappedInTransaction(): void {
36+
$driver = $this->create_in_memory_driver();
37+
$driver->query( 'CREATE TABLE t (id INT, name VARCHAR(255))' );
38+
39+
$driver->query( 'SELECT * FROM t' );
40+
41+
$this->assertStringStartsNotWith( 'BEGIN', $driver->get_last_sqlite_queries()[0]['sql'] );
42+
}
43+
44+
public function testShowQueryOpensReadOnlyTransaction(): void {
45+
$driver = $this->create_in_memory_driver();
46+
$driver->query( 'CREATE TABLE t (id INT, name VARCHAR(255))' );
47+
48+
$driver->query( 'SHOW TABLES' );
49+
50+
$this->assertSame( 'BEGIN', $driver->get_last_sqlite_queries()[0]['sql'] );
51+
}
52+
53+
public function testDescribeQueryOpensReadOnlyTransaction(): void {
54+
$driver = $this->create_in_memory_driver();
55+
$driver->query( 'CREATE TABLE t (id INT, name VARCHAR(255))' );
56+
57+
$driver->query( 'DESCRIBE t' );
58+
59+
$this->assertSame( 'BEGIN', $driver->get_last_sqlite_queries()[0]['sql'] );
60+
}
61+
62+
/**
63+
* @dataProvider provideWriteStatements
64+
*/
65+
public function testWriteQueryOpensWriteTransaction( string $query ): void {
66+
$driver = $this->create_in_memory_driver();
67+
$driver->query( 'CREATE TABLE t (id INT, name VARCHAR(255))' );
68+
$driver->query( "INSERT INTO t VALUES (1, 'Alice')" );
69+
70+
$driver->query( $query );
71+
72+
$this->assertSame( 'BEGIN IMMEDIATE', $driver->get_last_sqlite_queries()[0]['sql'] );
73+
}
74+
75+
public function provideWriteStatements(): array {
76+
return array(
77+
'INSERT' => array( "INSERT INTO t VALUES (2, 'Bob')" ),
78+
'UPDATE' => array( "UPDATE t SET name = 'Carol' WHERE id = 1" ),
79+
'DELETE' => array( 'DELETE FROM t WHERE id = 1' ),
80+
'REPLACE' => array( "REPLACE INTO t VALUES (1, 'Dan')" ),
81+
'CREATE TABLE' => array( 'CREATE TABLE u (id INT)' ),
82+
'ALTER TABLE' => array( 'ALTER TABLE t ADD COLUMN x INT' ),
83+
'DROP TABLE' => array( 'DROP TABLE t' ),
84+
'TRUNCATE TABLE' => array( 'TRUNCATE TABLE t' ),
85+
);
86+
}
87+
88+
public function testSelectQuerySucceedsWhileAnotherConnectionHoldsWriteLock(): void {
89+
$this->assertReadOnlyQuerySucceedsUnderWriteLock( 'SELECT * FROM t' );
90+
}
91+
92+
public function testShowQuerySucceedsWhileAnotherConnectionHoldsWriteLock(): void {
93+
$this->assertReadOnlyQuerySucceedsUnderWriteLock( 'SHOW TABLES' );
94+
}
95+
96+
public function testDescribeQuerySucceedsWhileAnotherConnectionHoldsWriteLock(): void {
97+
$this->assertReadOnlyQuerySucceedsUnderWriteLock( 'DESCRIBE t' );
98+
}
99+
100+
private function assertReadOnlyQuerySucceedsUnderWriteLock( string $query ): void {
101+
// Connection A: set up the database.
102+
$conn_a = new WP_SQLite_Connection( array( 'path' => $this->db_path ) );
103+
$driver_a = new WP_SQLite_Driver( $conn_a, 'wp' );
104+
$driver_a->query( 'CREATE TABLE t (id INT, name VARCHAR(255))' );
105+
$driver_a->query( "INSERT INTO t VALUES (1, 'Alice')" );
106+
107+
// Simulate another PHP process holding a write transaction.
108+
$conn_a->get_pdo()->exec( 'BEGIN IMMEDIATE' );
109+
110+
try {
111+
// Connection B with zero timeout — any lock conflict fails immediately.
112+
$conn_b = new WP_SQLite_Connection(
113+
array(
114+
'path' => $this->db_path,
115+
'timeout' => 0,
116+
)
117+
);
118+
$driver_b = new WP_SQLite_Driver( $conn_b, 'wp' );
119+
$conn_b->get_pdo()->setAttribute( PDO::ATTR_TIMEOUT, 0 );
120+
121+
$result = $driver_b->query( $query );
122+
123+
$this->assertIsArray( $result );
124+
$this->assertNotEmpty( $result );
125+
} finally {
126+
$conn_a->get_pdo()->exec( 'ROLLBACK' );
127+
}
128+
}
129+
130+
private function create_in_memory_driver(): WP_SQLite_Driver {
131+
$pdo_class = PHP_VERSION_ID >= 80400 ? PDO\SQLite::class : PDO::class;
132+
$pdo = new $pdo_class( 'sqlite::memory:' );
133+
$connection = new WP_SQLite_Connection( array( 'pdo' => $pdo ) );
134+
return new WP_SQLite_Driver( $connection, 'wp' );
135+
}
136+
}

0 commit comments

Comments
 (0)