diff --git a/CHANGES.rst b/CHANGES.rst index f3934db2..e8fffa20 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -11,6 +11,10 @@ Version 1.4 Unreleased +* Use SQLite's `write-ahead logging`_ to increase concurrency + and reduce "database is locked" errors. + At the moment there is no way to disable WAL. + (:issue:`169`, :issue:`175`) * Do not fail for feeds with incorrectly-declared media types, if feedparser can parse the feed; this is similar to the current behavior for incorrectly-declared encodings. diff --git a/src/reader/_sqlite_utils.py b/src/reader/_sqlite_utils.py index 1192c62e..b3cd6833 100644 --- a/src/reader/_sqlite_utils.py +++ b/src/reader/_sqlite_utils.py @@ -214,7 +214,7 @@ def get_version(db: sqlite3.Connection) -> Optional[int]: except sqlite3.OperationalError as e: if "no such table" in str(e).lower(): return None - raise + raise SchemaVersionError(f"cannot get current version: {e}") from e assert isinstance(version, int) return version @@ -292,12 +292,21 @@ def setup_db( migrations: Dict[int, _DBFunction], minimum_sqlite_version: Tuple[int, ...], required_sqlite_compile_options: Sequence[str] = (), + wal_enabled: Optional[bool] = None, ) -> None: require_version(db, minimum_sqlite_version) require_compile_options(db, required_sqlite_compile_options) db.execute("PRAGMA foreign_keys = ON;") + # Can't do this in a transaction, so we just do it all the time. + # https://github.com/lemon24/reader/issues/169 + if wal_enabled is not None: + if wal_enabled: + db.execute("PRAGMA journal_mode = WAL;") + else: + db.execute("PRAGMA journal_mode = DELETE;") + migration = HeavyMigration(create, version, migrations) migration.migrate(db) diff --git a/src/reader/_storage.py b/src/reader/_storage.py index 21b780a0..e6b341a4 100644 --- a/src/reader/_storage.py +++ b/src/reader/_storage.py @@ -142,7 +142,7 @@ def update_from_18_to_19(db: sqlite3.Connection) -> None: # pragma: no cover db.execute("ALTER TABLE feeds ADD COLUMN last_exception TEXT;") -def setup_db(db: sqlite3.Connection) -> None: +def setup_db(db: sqlite3.Connection, wal_enabled: Optional[bool]) -> None: return setup_sqlite_db( db, create=create_db, @@ -157,6 +157,7 @@ def setup_db(db: sqlite3.Connection) -> None: minimum_sqlite_version=(3, 15), # We use the JSON1 extension for entries.content. required_sqlite_compile_options=["ENABLE_JSON1"], + wal_enabled=wal_enabled, ) @@ -180,7 +181,12 @@ class Storage: recent_threshold = timedelta(7) @wrap_exceptions(StorageError) - def __init__(self, path: str, timeout: Optional[float] = None): + def __init__( + self, + path: str, + timeout: Optional[float] = None, + wal_enabled: Optional[bool] = True, + ): kwargs = {} if timeout is not None: kwargs['timeout'] = timeout @@ -188,7 +194,7 @@ def __init__(self, path: str, timeout: Optional[float] = None): db = self.connect(path, detect_types=sqlite3.PARSE_DECLTYPES, **kwargs) try: try: - self.setup_db(db) + self.setup_db(db, wal_enabled) except BaseException: db.close() raise diff --git a/tests/test_sqlite_utils.py b/tests/test_sqlite_utils.py index e75fcc6e..c666018f 100644 --- a/tests/test_sqlite_utils.py +++ b/tests/test_sqlite_utils.py @@ -10,6 +10,7 @@ from reader._sqlite_utils import require_version from reader._sqlite_utils import RequirementError from reader._sqlite_utils import SchemaVersionError +from reader._sqlite_utils import setup_db from reader._sqlite_utils import wrap_exceptions @@ -215,6 +216,15 @@ def test_migration_invalid_version(): assert columns == ['one', 'two'] +def test_migration_invalid_version_table(): + db = sqlite3.connect(':memory:') + with db: + db.execute("CREATE TABLE version (not_version);") + migration = HeavyMigration(create_db_2, 2, {}) + with pytest.raises(SchemaVersionError) as excinfo: + migration.migrate(db) + + def test_migration_integrity_error(): def create_db(db): db.execute("CREATE TABLE t (one INTEGER PRIMARY KEY);") @@ -265,3 +275,23 @@ def test_require_compile_options(): # shouldn't raise an exception require_compile_options(db, ['ONE']) require_compile_options(db, ['ONE', 'TWO']) + + +@pytest.mark.parametrize( + 'wal_enabled, expected_mode', [(None, 'memory'), (True, 'wal'), (False, 'delete')] +) +def test_setup_db_wal_enabled(db_path, wal_enabled, expected_mode): + db = sqlite3.connect(db_path) + db.execute("PRAGMA journal_mode = MEMORY;") + + setup_db( + db, + create=lambda db: None, + version=0, + migrations={}, + minimum_sqlite_version=(3, 15, 0), + wal_enabled=wal_enabled, + ) + + ((mode,),) = db.execute("PRAGMA journal_mode;") + assert mode.lower() == expected_mode diff --git a/tests/test_storage.py b/tests/test_storage.py index 54d1efad..cbe2bc28 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -181,7 +181,8 @@ def test_errors_locked(db_path, do_stuff): def check_errors_locked(db_path, pre_stuff, do_stuff, exc_type): """Actual implementation of test_errors_locked, so it can be reused.""" - storage = Storage(db_path) + # WAL provides more concurrency; some things won't to block with it enabled. + storage = Storage(db_path, wal_enabled=False) storage.db.execute("PRAGMA busy_timeout = 0;") feed = FeedData('one') @@ -195,7 +196,7 @@ def check_errors_locked(db_path, pre_stuff, do_stuff, exc_type): can_return_from_transaction = threading.Event() def target(): - storage = Storage(db_path) + storage = Storage(db_path, wal_enabled=False) storage.db.isolation_level = None storage.db.execute("BEGIN EXCLUSIVE;") in_transaction.set() @@ -272,7 +273,8 @@ def test_iter_locked(db_path, iter_stuff): def check_iter_locked(db_path, pre_stuff, iter_stuff): """Actual implementation of test_errors_locked, so it can be reused.""" - storage = Storage(db_path) + # WAL provides more concurrency; some things won't to block with it enabled. + storage = Storage(db_path, wal_enabled=False) feed = FeedData('one') entry = EntryData('one', 'entry', datetime(2010, 1, 1), title='entry') @@ -296,7 +298,7 @@ def check_iter_locked(db_path, pre_stuff, iter_stuff): next(rv) # shouldn't raise an exception - storage = Storage(db_path, timeout=0) + storage = Storage(db_path, timeout=0, wal_enabled=False) storage.mark_as_read_unread(feed.url, entry.id, 1) storage = Storage(db_path, timeout=0) storage.mark_as_read_unread(feed.url, entry.id, 0)