Skip to content

Commit

Permalink
Use WAL, always (for now).
Browse files Browse the repository at this point in the history
For #169 / #175.
  • Loading branch information
lemon24 committed Jun 30, 2020
1 parent ef8e600 commit 4a6de7f
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 10 additions & 1 deletion src/reader/_sqlite_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)

Expand Down
12 changes: 9 additions & 3 deletions src/reader/_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
)


Expand All @@ -180,15 +181,20 @@ 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

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
Expand Down
30 changes: 30 additions & 0 deletions tests/test_sqlite_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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);")
Expand Down Expand Up @@ -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
10 changes: 6 additions & 4 deletions tests/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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()
Expand Down Expand Up @@ -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')
Expand All @@ -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)
Expand Down

0 comments on commit 4a6de7f

Please sign in to comment.