refactor(state): topic rebuild goes through _rebuild_table; heal also swallows a replaced state.db
The hand-rolled CREATE/INSERT/DROP/RENAME duplicated SessionSchemaMixin._rebuild_table,
which already runs inside a caller-owned transaction and has its own atomicity test.
The stranded {table}_new drop stays: older builds crashed with that scratch name.
_execute_write probes the DB generation before running the callback and raises
StateDbReplacedError (a RuntimeError, not sqlite3.Error); _read_ctx never does, so
the heal must not let it escape a read path that promises the empty value.
Atomicity probe now denies the copy into the fresh table: a non-transactional
rebuild leaves an empty v3 table the retry skips (verified red with executescript).
This commit is contained in:
@@ -9,6 +9,7 @@ import time
|
||||
from typing import Any, Dict, List, Optional
|
||||
|
||||
from hermes_state_common import _PREVIEW_ELIGIBLE_SQL, _PREVIEW_RAW_SELECT, _sql_session_last_active
|
||||
from hermes_state_errors import StateDbReplacedError
|
||||
|
||||
# caplog tests pin the "hermes_state" logger name.
|
||||
logger = logging.getLogger("hermes_state")
|
||||
@@ -111,7 +112,7 @@ class SessionTelegramTopicsMixin:
|
||||
return empty
|
||||
try:
|
||||
self.apply_telegram_topic_migration()
|
||||
except sqlite3.Error:
|
||||
except (sqlite3.Error, StateDbReplacedError):
|
||||
logger.warning("telegram topic tables are pre-v3 and the heal failed; reading as empty", exc_info=True)
|
||||
return empty
|
||||
return read()
|
||||
@@ -139,22 +140,18 @@ class SessionTelegramTopicsMixin:
|
||||
if "profile_name" in have:
|
||||
continue
|
||||
# v1/v2 → v3. SQLite can't ALTER a PK or FK, so rebuild (also supplies v2's
|
||||
# ON DELETE CASCADE). Legacy rows land in "default" only. execute(), not
|
||||
# executescript(): executescript COMMITs the open BEGIN IMMEDIATE, so a crash after
|
||||
# the DROP strands rows in {table}_new (#42004). A {table}_new left by such a crash
|
||||
# on an older build is dropped first; the legacy table is still intact to re-copy.
|
||||
# ON DELETE CASCADE); _rebuild_table runs inside the open BEGIN IMMEDIATE so a crash
|
||||
# rolls back instead of stranding rows (#42004). A {table}_new left by an older
|
||||
# build's executescript crash is dropped first; its legacy table is still intact.
|
||||
# v1 bindings had no ON DELETE CASCADE, so pruned sessions left orphan rows that
|
||||
# the v3 FK (foreign_keys=ON on the writer) would reject — copy only live ones.
|
||||
legacy_columns = columns.replace("profile_name, ", "", 1)
|
||||
live = " WHERE EXISTS (SELECT 1 FROM sessions s WHERE s.id = session_id)" if "session_id" in columns else ""
|
||||
conn.execute(f"DROP TABLE IF EXISTS {table}_new")
|
||||
conn.execute(f"CREATE TABLE {table}_new ({ddl})")
|
||||
conn.execute(
|
||||
f"INSERT INTO {table}_new ({columns}) "
|
||||
f"SELECT 'default', {legacy_columns} FROM {table}{live}"
|
||||
self._rebuild_table(
|
||||
conn.cursor(), table, f"{table}_legacy", f"CREATE TABLE {table} ({ddl})",
|
||||
f"INSERT INTO {table} ({columns}) SELECT 'default', {legacy_columns} FROM {table}_legacy{live}",
|
||||
)
|
||||
conn.execute(f"DROP TABLE {table}")
|
||||
conn.execute(f"ALTER TABLE {table}_new RENAME TO {table}")
|
||||
# Indexes after any rebuild: the user index needs profile_name.
|
||||
conn.execute(
|
||||
"CREATE UNIQUE INDEX IF NOT EXISTS idx_telegram_dm_topic_bindings_session "
|
||||
|
||||
@@ -101,27 +101,21 @@ def test_absent_tables_still_read_empty_and_are_not_created(tmp_path: Path):
|
||||
|
||||
|
||||
def test_heal_rebuild_rolls_back_atomically(tmp_path: Path):
|
||||
"""The v2→v3 rebuild must stay inside _execute_write's transaction.
|
||||
|
||||
Pre-fix the rebuild ran via executescript, which implicitly commits the
|
||||
open transaction and then autocommits each statement: a failure once the
|
||||
rebuild reaches the DROP strands legacy rows in {table}_new (#42004
|
||||
diagnosed the same shape for the v1→v2 rebuild) and the retry then builds
|
||||
an empty v3 table. The same mid-rebuild failure must now roll back to the
|
||||
intact v2 table so the next read can heal cleanly.
|
||||
"""
|
||||
"""The v2→v3 rebuild must stay inside _execute_write's transaction: a mid-rebuild
|
||||
failure leaves the intact v2 table (and its version stamp) for the next read to heal,
|
||||
never a half-built v3 table the retry would mistake for a finished one (#42004)."""
|
||||
db_path = tmp_path / "v2-upgrade.db"
|
||||
_create_v2_state(db_path)
|
||||
db = SessionDB(db_path=db_path)
|
||||
|
||||
def deny_topic_rebuild_rename(action, arg1, arg2, db_name, trigger):
|
||||
# Fail the rebuild's LAST statement, after the legacy table is already dropped: only a
|
||||
# transactional rebuild still has the rows at that point.
|
||||
if action == sqlite3.SQLITE_ALTER_TABLE and arg2 == "telegram_dm_topic_mode_new":
|
||||
def deny_topic_rebuild_copy(action, arg1, arg2, db_name, trigger):
|
||||
# Fail the copy into the fresh table, after the live table was renamed away: a
|
||||
# non-transactional rebuild leaves an empty v3 table behind and the retry skips it.
|
||||
if action == sqlite3.SQLITE_INSERT and arg1 == "telegram_dm_topic_mode":
|
||||
return sqlite3.SQLITE_DENY
|
||||
return sqlite3.SQLITE_OK
|
||||
|
||||
db._conn.set_authorizer(deny_topic_rebuild_rename)
|
||||
db._conn.set_authorizer(deny_topic_rebuild_copy)
|
||||
try:
|
||||
# The guarded read degrades to "off" instead of raising...
|
||||
assert not db.is_telegram_topic_mode_enabled(
|
||||
@@ -130,8 +124,8 @@ def test_heal_rebuild_rolls_back_atomically(tmp_path: Path):
|
||||
finally:
|
||||
db._conn.set_authorizer(None)
|
||||
|
||||
# ...and the interrupted rebuild rolled back completely: the dropped original is back
|
||||
# (otherwise the retry below builds an empty v3 table and reads as off) and so is the version stamp.
|
||||
# ...and the interrupted rebuild rolled back completely: the renamed-away original is back
|
||||
# (otherwise the retry below finds no legacy table, builds an empty v3 one and reads as off).
|
||||
assert db.get_meta("telegram_dm_topic_schema_version") == "2"
|
||||
# Idempotent under retry (the _execute_write contract): the next clean
|
||||
# read heals and keeps the legacy rows.
|
||||
|
||||
Reference in New Issue
Block a user