From 5dbcb64ebb625bfe5c4c4f794a08157cf2cc5be5 Mon Sep 17 00:00:00 2001 From: Ralf Kistner Date: Tue, 2 Apr 2024 12:05:41 +0200 Subject: [PATCH 1/6] Use busy-timeout for database-level locks. --- lib/src/sqlite_connection_impl.dart | 25 +++++----------------- lib/src/sqlite_open_factory.dart | 22 +++++++++++++++++-- lib/src/sqlite_options.dart | 11 ++++++++-- test/basic_test.dart | 33 +++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 24 deletions(-) diff --git a/lib/src/sqlite_connection_impl.dart b/lib/src/sqlite_connection_impl.dart index 643152e..9fffa08 100644 --- a/lib/src/sqlite_connection_impl.dart +++ b/lib/src/sqlite_connection_impl.dart @@ -123,30 +123,15 @@ class SqliteConnectionImpl with SqliteQueries implements SqliteConnection { @override Future writeLock(Future Function(SqliteWriteContext tx) callback, {Duration? lockTimeout, String? debugContext}) async { - final stopWatch = lockTimeout == null ? null : (Stopwatch()..start()); // Private lock to synchronize this with other statements on the same connection, // to ensure that transactions aren't interleaved. return await _connectionMutex.lock(() async { - Duration? innerTimeout; - if (lockTimeout != null && stopWatch != null) { - innerTimeout = lockTimeout - stopWatch.elapsed; - stopWatch.stop(); + final ctx = _TransactionContext(_isolateClient); + try { + return await callback(ctx); + } finally { + await ctx.close(); } - // DB lock so that only one write happens at a time - return await _writeMutex.lock(() async { - final ctx = _TransactionContext(_isolateClient); - try { - return await callback(ctx); - } finally { - await ctx.close(); - } - }, timeout: innerTimeout).catchError((error, stackTrace) { - if (error is TimeoutException) { - return Future.error(TimeoutException( - 'Failed to acquire global write lock', lockTimeout)); - } - return Future.error(error, stackTrace); - }); }, timeout: lockTimeout); } } diff --git a/lib/src/sqlite_open_factory.dart b/lib/src/sqlite_open_factory.dart index 4f1845a..97f1647 100644 --- a/lib/src/sqlite_open_factory.dart +++ b/lib/src/sqlite_open_factory.dart @@ -1,6 +1,6 @@ import 'dart:async'; -import 'package:sqlite3/sqlite3.dart' as sqlite; +import 'package:sqlite_async/sqlite3.dart' as sqlite; import 'sqlite_options.dart'; @@ -29,6 +29,11 @@ class DefaultSqliteOpenFactory implements SqliteOpenFactory { List pragmaStatements(SqliteOpenOptions options) { List statements = []; + if (sqliteOptions.busyTimeout != null) { + statements.add( + 'PRAGMA busy_timeout = ${sqliteOptions.busyTimeout!.inMilliseconds}'); + } + if (options.primaryConnection && sqliteOptions.journalMode != null) { // Persisted - only needed on the primary connection statements @@ -51,8 +56,21 @@ class DefaultSqliteOpenFactory implements SqliteOpenFactory { final mode = options.openMode; var db = sqlite.sqlite3.open(path, mode: mode, mutex: false); + // Pragma statements don't have the same BUSY_TIMEOUT behavior as normal statements. + // We add a manual retry loop for those. for (var statement in pragmaStatements(options)) { - db.execute(statement); + for (var tries = 0; tries < 30; tries++) { + try { + db.execute(statement); + break; + } on sqlite.SqliteException catch (e) { + if (e.resultCode == sqlite.SqlError.SQLITE_BUSY && tries < 29) { + continue; + } else { + rethrow; + } + } + } } return db; } diff --git a/lib/src/sqlite_options.dart b/lib/src/sqlite_options.dart index 36beb7c..87d7ee8 100644 --- a/lib/src/sqlite_options.dart +++ b/lib/src/sqlite_options.dart @@ -11,15 +11,22 @@ class SqliteOptions { /// attempt to truncate the file afterwards. final int? journalSizeLimit; + /// Timeout waiting for locks to be released by other write connections. + /// Defaults to 30 seconds. + /// Set to 0 to fail immediately when the database is locked. + final Duration? busyTimeout; + const SqliteOptions.defaults() : journalMode = SqliteJournalMode.wal, journalSizeLimit = 6 * 1024 * 1024, // 1.5x the default checkpoint size - synchronous = SqliteSynchronous.normal; + synchronous = SqliteSynchronous.normal, + busyTimeout = const Duration(seconds: 30); const SqliteOptions( {this.journalMode = SqliteJournalMode.wal, this.journalSizeLimit = 6 * 1024 * 1024, - this.synchronous = SqliteSynchronous.normal}); + this.synchronous = SqliteSynchronous.normal, + this.busyTimeout = const Duration(seconds: 30)}); } /// SQLite journal mode. Set on the primary connection. diff --git a/test/basic_test.dart b/test/basic_test.dart index 9b563c8..55a7d12 100644 --- a/test/basic_test.dart +++ b/test/basic_test.dart @@ -64,6 +64,39 @@ void main() { } }); + test('Concurrency 2', () async { + final db1 = + SqliteDatabase.withFactory(testFactory(path: path), maxReaders: 3); + + final db2 = + SqliteDatabase.withFactory(testFactory(path: path), maxReaders: 3); + await db1.initialize(); + await createTables(db1); + await db2.initialize(); + print("${DateTime.now()} start"); + + var futures1 = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11].map((i) { + return db1.execute( + "INSERT OR REPLACE INTO test_data(id, description) SELECT ? as i, test_sleep(?) || ' ' || test_connection_name() || ' 1 ' || datetime() as connection RETURNING *", + [ + i, + 5 + Random().nextInt(20) + ]).then((value) => print("${DateTime.now()} $value")); + }).toList(); + + var futures2 = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11].map((i) { + return db2.execute( + "INSERT OR REPLACE INTO test_data(id, description) SELECT ? as i, test_sleep(?) || ' ' || test_connection_name() || ' 2 ' || datetime() as connection RETURNING *", + [ + i, + 5 + Random().nextInt(20) + ]).then((value) => print("${DateTime.now()} $value")); + }).toList(); + await Future.wait(futures1); + await Future.wait(futures2); + print("${DateTime.now()} done"); + }); + test('read-only transactions', () async { final db = await setupDatabase(path: path); await createTables(db); From b8a68f9d1f5db4e01d5b8679af8c094b484db540 Mon Sep 17 00:00:00 2001 From: Ralf Kistner Date: Tue, 2 Apr 2024 14:38:37 +0200 Subject: [PATCH 2/6] Revert mutex change. --- lib/src/sqlite_connection_impl.dart | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/src/sqlite_connection_impl.dart b/lib/src/sqlite_connection_impl.dart index 9fffa08..643152e 100644 --- a/lib/src/sqlite_connection_impl.dart +++ b/lib/src/sqlite_connection_impl.dart @@ -123,15 +123,30 @@ class SqliteConnectionImpl with SqliteQueries implements SqliteConnection { @override Future writeLock(Future Function(SqliteWriteContext tx) callback, {Duration? lockTimeout, String? debugContext}) async { + final stopWatch = lockTimeout == null ? null : (Stopwatch()..start()); // Private lock to synchronize this with other statements on the same connection, // to ensure that transactions aren't interleaved. return await _connectionMutex.lock(() async { - final ctx = _TransactionContext(_isolateClient); - try { - return await callback(ctx); - } finally { - await ctx.close(); + Duration? innerTimeout; + if (lockTimeout != null && stopWatch != null) { + innerTimeout = lockTimeout - stopWatch.elapsed; + stopWatch.stop(); } + // DB lock so that only one write happens at a time + return await _writeMutex.lock(() async { + final ctx = _TransactionContext(_isolateClient); + try { + return await callback(ctx); + } finally { + await ctx.close(); + } + }, timeout: innerTimeout).catchError((error, stackTrace) { + if (error is TimeoutException) { + return Future.error(TimeoutException( + 'Failed to acquire global write lock', lockTimeout)); + } + return Future.error(error, stackTrace); + }); }, timeout: lockTimeout); } } From 713cb128c2232e6fcfe02c3d942040a12cd0c6b6 Mon Sep 17 00:00:00 2001 From: Ralf Kistner Date: Tue, 2 Apr 2024 14:41:00 +0200 Subject: [PATCH 3/6] Rename to lockTimeout. --- lib/src/sqlite_open_factory.dart | 5 +++-- lib/src/sqlite_options.dart | 10 +++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/src/sqlite_open_factory.dart b/lib/src/sqlite_open_factory.dart index 97f1647..b4779ef 100644 --- a/lib/src/sqlite_open_factory.dart +++ b/lib/src/sqlite_open_factory.dart @@ -29,9 +29,10 @@ class DefaultSqliteOpenFactory implements SqliteOpenFactory { List pragmaStatements(SqliteOpenOptions options) { List statements = []; - if (sqliteOptions.busyTimeout != null) { + if (sqliteOptions.lockTimeout != null) { + // May be replaced by a Dart-level retry mechanism in the future statements.add( - 'PRAGMA busy_timeout = ${sqliteOptions.busyTimeout!.inMilliseconds}'); + 'PRAGMA busy_timeout = ${sqliteOptions.lockTimeout!.inMilliseconds}'); } if (options.primaryConnection && sqliteOptions.journalMode != null) { diff --git a/lib/src/sqlite_options.dart b/lib/src/sqlite_options.dart index 87d7ee8..9602fdd 100644 --- a/lib/src/sqlite_options.dart +++ b/lib/src/sqlite_options.dart @@ -11,22 +11,22 @@ class SqliteOptions { /// attempt to truncate the file afterwards. final int? journalSizeLimit; - /// Timeout waiting for locks to be released by other write connections. + /// Timeout waiting for locks to be released by other connections. /// Defaults to 30 seconds. - /// Set to 0 to fail immediately when the database is locked. - final Duration? busyTimeout; + /// Set to null or [Duration.zero] to fail immediately when the database is locked. + final Duration? lockTimeout; const SqliteOptions.defaults() : journalMode = SqliteJournalMode.wal, journalSizeLimit = 6 * 1024 * 1024, // 1.5x the default checkpoint size synchronous = SqliteSynchronous.normal, - busyTimeout = const Duration(seconds: 30); + lockTimeout = const Duration(seconds: 30); const SqliteOptions( {this.journalMode = SqliteJournalMode.wal, this.journalSizeLimit = 6 * 1024 * 1024, this.synchronous = SqliteSynchronous.normal, - this.busyTimeout = const Duration(seconds: 30)}); + this.lockTimeout = const Duration(seconds: 30)}); } /// SQLite journal mode. Set on the primary connection. From b8121c2ccd4981e6559f27021957c9a46e3f5c11 Mon Sep 17 00:00:00 2001 From: Ralf Kistner Date: Tue, 2 Apr 2024 16:43:39 +0200 Subject: [PATCH 4/6] Bump dart SDK. --- .github/workflows/test.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index c9fa2df..f3882c0 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -32,19 +32,19 @@ jobs: include: - sqlite_version: "3440200" sqlite_url: "https://www.sqlite.org/2023/sqlite-autoconf-3440200.tar.gz" - dart_sdk: 3.2.4 + dart_sdk: 3.3.3 - sqlite_version: "3430200" sqlite_url: "https://www.sqlite.org/2023/sqlite-autoconf-3430200.tar.gz" - dart_sdk: 3.2.4 + dart_sdk: 3.3.3 - sqlite_version: "3420000" sqlite_url: "https://www.sqlite.org/2023/sqlite-autoconf-3420000.tar.gz" - dart_sdk: 3.2.4 + dart_sdk: 3.3.3 - sqlite_version: "3410100" sqlite_url: "https://www.sqlite.org/2023/sqlite-autoconf-3410100.tar.gz" - dart_sdk: 3.2.4 + dart_sdk: 3.3.3 - sqlite_version: "3380000" sqlite_url: "https://www.sqlite.org/2022/sqlite-autoconf-3380000.tar.gz" - dart_sdk: 3.2.0 + dart_sdk: 3.3.3 steps: - uses: actions/checkout@v3 - uses: dart-lang/setup-dart@v1 From 28cf2667427cf5da8b4faa12f11c9fc6e3767054 Mon Sep 17 00:00:00 2001 From: Ralf Kistner Date: Wed, 3 Apr 2024 09:19:07 +0200 Subject: [PATCH 5/6] Clarify docs on multiple instances. --- lib/src/sqlite_database.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/src/sqlite_database.dart b/lib/src/sqlite_database.dart index c937743..d792092 100644 --- a/lib/src/sqlite_database.dart +++ b/lib/src/sqlite_database.dart @@ -15,8 +15,10 @@ import 'update_notification.dart'; /// A SQLite database instance. /// -/// Use one instance per database file. If multiple instances are used, update -/// notifications may not trigger, and calls may fail with "SQLITE_BUSY" errors. +/// Use one instance per database file where feasible. +/// +/// If multiple instances are used, update notifications will not be propagated between them. +/// For update notifications across isolates, use [isolateConnectionFactory]. class SqliteDatabase with SqliteQueries implements SqliteConnection { /// The maximum number of concurrent read transactions if not explicitly specified. static const int defaultMaxReaders = 5; From ea27d31e76a18ab86ae4143e6b860d898c8bcf9a Mon Sep 17 00:00:00 2001 From: Ralf Kistner Date: Thu, 4 Apr 2024 14:23:47 +0200 Subject: [PATCH 6/6] Add notes explaining the different locks. --- lib/src/sqlite_connection.dart | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/src/sqlite_connection.dart b/lib/src/sqlite_connection.dart index 156f967..1fc5525 100644 --- a/lib/src/sqlite_connection.dart +++ b/lib/src/sqlite_connection.dart @@ -84,7 +84,8 @@ abstract class SqliteConnection extends SqliteWriteContext { /// Open a read-write transaction. /// /// This takes a global lock - only one write transaction can execute against - /// the database at a time. + /// the database at a time. This applies even when constructing separate + /// [SqliteDatabase] instances for the same database file. /// /// Statements within the transaction must be done on the provided /// [SqliteWriteContext] - attempting statements on the [SqliteConnection] @@ -104,6 +105,9 @@ abstract class SqliteConnection extends SqliteWriteContext { /// Takes a read lock, without starting a transaction. /// + /// The lock only applies to a single [SqliteConnection], and multiple + /// connections may hold read locks at the same time. + /// /// In most cases, [readTransaction] should be used instead. Future readLock(Future Function(SqliteReadContext tx) callback, {Duration? lockTimeout, String? debugContext}); @@ -111,6 +115,10 @@ abstract class SqliteConnection extends SqliteWriteContext { /// Takes a global lock, without starting a transaction. /// /// In most cases, [writeTransaction] should be used instead. + /// + /// The lock applies to all [SqliteConnection] instances for a [SqliteDatabase]. + /// Locks for separate [SqliteDatabase] instances on the same database file + /// may be held concurrently. Future writeLock(Future Function(SqliteWriteContext tx) callback, {Duration? lockTimeout, String? debugContext});