perf(fts): drain trash tables with a high-water marker instead of re-scanning
_fts_teardown_trash_step deleted rows via 'WHERE key IN (SELECT key LIMIT N)' — each chunk's subquery re-scanned from the start of the table, so chunk k skipped past (k-1)xN already-deleted rows: O(n²) total row visits. On a v22 shadow table with ~230K rows that is on the order of 10^8 row visits, turning optimize-storage teardown into a multi-hour grind on slow disks, with a write lock held per chunk. Single-column INTEGER-PK trash tables now drain via a fts_teardown_<tbl>_progress high-water marker mirroring fts_rebuild_step: each chunk claims rows past the marker (SELECT ... WHERE key > ? ORDER BY key LIMIT N), deletes the claimed range, and publishes the new marker in the same transaction. Per-chunk work is bounded → O(n) total. TEXT-PK tables (the FTS config shadow table, pk like 'version') and compound-key tables fall back to the legacy chunked delete — those are small by construction. Fixes #79324
This commit is contained in:
+63
-2
@@ -164,6 +164,16 @@ class SessionSearchMixin:
|
||||
The trash tables are PLAIN tables (their vtable parent was demoted
|
||||
away during the migration), so chunked DELETE + final DROP involve
|
||||
no FTS5 machinery at all. Returns True while teardown work remains.
|
||||
|
||||
Single-column-key trash tables (the common shape — FTS shadow
|
||||
tables carry a rowid/integer PK) are drained with a high-water
|
||||
marker mirroring :meth:`fts_rebuild_step`: each chunk deletes only
|
||||
rows after the previously-drained key, so the per-chunk scan is
|
||||
bounded instead of re-scanning from the start of the table every
|
||||
chunk (O(n²) total on large trash tables, #79324). Compound-key
|
||||
trash tables (multi-column PK) cannot use a scalar high-water
|
||||
comparison, so they keep the legacy chunked ``LIMIT`` delete —
|
||||
those shadow tables are small by construction.
|
||||
"""
|
||||
with self._lock:
|
||||
trash = [
|
||||
@@ -179,11 +189,62 @@ class SessionSearchMixin:
|
||||
tbl = trash[0]
|
||||
|
||||
def _do(conn):
|
||||
pk_cols = [
|
||||
r[1] for r in conn.execute(f"PRAGMA table_info({tbl})")
|
||||
pk_info = [
|
||||
(r[1], (r[2] or "").upper())
|
||||
for r in conn.execute(f"PRAGMA table_info({tbl})")
|
||||
if r[5] > 0
|
||||
]
|
||||
pk_cols = [name for name, _typ in pk_info]
|
||||
key = ", ".join(pk_cols) if pk_cols else "rowid"
|
||||
|
||||
if len(pk_cols) == 1 and (not pk_info or pk_info[0][1] == "INTEGER"):
|
||||
# High-water drain: delete only rows past the marker key.
|
||||
# The marker is read/written inside the same BEGIN IMMEDIATE
|
||||
# transaction as the DELETE, so concurrent callers claim
|
||||
# disjoint key ranges instead of re-deleting. Only integer
|
||||
# PKs can anchor a numeric high-water comparison — the FTS
|
||||
# config shadow table (TEXT pk like 'version') falls back to
|
||||
# the legacy chunked delete below.
|
||||
marker_key = f"fts_teardown_{tbl}_progress"
|
||||
row = conn.execute(
|
||||
"SELECT value FROM state_meta WHERE key = ?",
|
||||
(marker_key,),
|
||||
).fetchone()
|
||||
high_water = int(row[0]) if row is not None else 0
|
||||
|
||||
# Claim the chunk's upper bound: the LAST row of the
|
||||
# LIMIT window, so a full chunk is deleted per step.
|
||||
upper_rows = conn.execute(
|
||||
f"SELECT {key} FROM {tbl} WHERE {key} > ? "
|
||||
f"ORDER BY {key} LIMIT {self._FTS_REBUILD_CHUNK_ROWS}",
|
||||
(high_water,),
|
||||
).fetchall()
|
||||
if not upper_rows:
|
||||
# Drained — the DROP is cheap now.
|
||||
conn.execute(f"DROP TABLE IF EXISTS {tbl}")
|
||||
conn.execute(
|
||||
"DELETE FROM state_meta WHERE key = ?", (marker_key,)
|
||||
)
|
||||
logger.info("Old FTS shadow table %s torn down.", tbl)
|
||||
return True
|
||||
|
||||
upper = upper_rows[-1][0]
|
||||
cur = conn.execute(
|
||||
f"DELETE FROM {tbl} WHERE {key} > ? AND {key} <= ?",
|
||||
(high_water, upper),
|
||||
)
|
||||
if cur.rowcount > 0:
|
||||
conn.execute(
|
||||
"INSERT INTO state_meta (key, value) VALUES (?, ?) "
|
||||
"ON CONFLICT(key) DO UPDATE SET value = excluded.value",
|
||||
(marker_key, str(upper)),
|
||||
)
|
||||
return True
|
||||
|
||||
# Compound-key or rowid trash table: legacy chunked delete.
|
||||
# These shadow tables are small, so the quadratic re-scan is
|
||||
# not a concern (#79324 keeps the high-water path for the big
|
||||
# single-key tables).
|
||||
cur = conn.execute(
|
||||
f"DELETE FROM {tbl} WHERE ({key}) IN "
|
||||
f"(SELECT {key} FROM {tbl} LIMIT {self._FTS_REBUILD_CHUNK_ROWS})"
|
||||
|
||||
@@ -3037,6 +3037,120 @@ class TestFTSExternalContentMigration:
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
def test_fts_teardown_single_key_high_water_drains_and_drops(self, tmp_path):
|
||||
"""#79324: single-column-key trash tables drain via a high-water
|
||||
marker so each chunk only scans rows after the previous chunk.
|
||||
|
||||
Builds a large trash table with a rowid-like integer PK, then drives
|
||||
``_fts_teardown_trash_step`` to completion. Verifies every row is
|
||||
removed, the marker advances monotonically, the marker is cleared,
|
||||
and the table is dropped at the end.
|
||||
"""
|
||||
db = SessionDB(db_path=tmp_path / "trash.db")
|
||||
try:
|
||||
conn = db._conn
|
||||
# A plain trash table shaped like a demoted FTS shadow table
|
||||
# (single integer PK — the common, large-table shape).
|
||||
conn.execute(
|
||||
"CREATE TABLE fts_v22_trash_messages_fts_data "
|
||||
"(docid INTEGER PRIMARY KEY, block BLOB)"
|
||||
)
|
||||
conn.executemany(
|
||||
"INSERT INTO fts_v22_trash_messages_fts_data "
|
||||
"(docid, block) VALUES (?, ?)",
|
||||
[(i, b"x" * 64) for i in range(1, 2501)],
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
assert db._has_fts_trash(conn) is True
|
||||
|
||||
steps = 0
|
||||
while db._fts_teardown_trash_step():
|
||||
steps += 1
|
||||
assert steps < 100, "teardown never finished"
|
||||
|
||||
# All rows gone, table dropped, marker cleaned up.
|
||||
assert db._has_fts_trash(conn) is False
|
||||
assert conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE name = "
|
||||
"'fts_v22_trash_messages_fts_data'"
|
||||
).fetchone() is None
|
||||
assert db.get_meta("fts_teardown_fts_v22_trash_messages_fts_data_progress") is None
|
||||
# Multiple chunks were needed (2500 rows / 500 chunk).
|
||||
assert steps >= 5
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
def test_fts_teardown_high_water_resumes_after_interruption(self, tmp_path):
|
||||
"""#79324: the high-water marker survives an interrupted teardown,
|
||||
so the next call resumes from the marker instead of the table start."""
|
||||
db = SessionDB(db_path=tmp_path / "trash.db")
|
||||
try:
|
||||
conn = db._conn
|
||||
conn.execute(
|
||||
"CREATE TABLE fts_v22_trash_messages_fts_data "
|
||||
"(docid INTEGER PRIMARY KEY, block BLOB)"
|
||||
)
|
||||
conn.executemany(
|
||||
"INSERT INTO fts_v22_trash_messages_fts_data "
|
||||
"(docid, block) VALUES (?, ?)",
|
||||
[(i, b"x" * 64) for i in range(1, 1201)],
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
# Drain two chunks, then simulate a crash: the marker stays at
|
||||
# the last drained key and the remaining rows are intact.
|
||||
assert db._fts_teardown_trash_step() is True
|
||||
assert db._fts_teardown_trash_step() is True
|
||||
marker = db.get_meta("fts_teardown_fts_v22_trash_messages_fts_data_progress")
|
||||
assert marker is not None
|
||||
assert int(marker) == 1000 # 2 chunks x 500 rows
|
||||
|
||||
remaining = conn.execute(
|
||||
"SELECT COUNT(*) FROM fts_v22_trash_messages_fts_data"
|
||||
).fetchone()[0]
|
||||
assert remaining == 200
|
||||
|
||||
# Resume: drains the rest, drops the table.
|
||||
while db._fts_teardown_trash_step():
|
||||
pass
|
||||
assert db._has_fts_trash(conn) is False
|
||||
assert db.get_meta("fts_teardown_fts_v22_trash_messages_fts_data_progress") is None
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
def test_fts_teardown_compound_key_keeps_legacy_path(self, tmp_path):
|
||||
"""#79324: multi-column-PK trash tables (small by construction) keep
|
||||
the legacy chunked delete — the high-water path only applies to
|
||||
single-column keys."""
|
||||
db = SessionDB(db_path=tmp_path / "trash.db")
|
||||
try:
|
||||
conn = db._conn
|
||||
conn.execute(
|
||||
"CREATE TABLE fts_v22_trash_messages_fts_idx "
|
||||
"(segid INTEGER, term TEXT, pgno INTEGER, "
|
||||
"PRIMARY KEY (segid, term, pgno)) WITHOUT ROWID"
|
||||
)
|
||||
conn.executemany(
|
||||
"INSERT INTO fts_v22_trash_messages_fts_idx "
|
||||
"(segid, term, pgno) VALUES (?, ?, ?)",
|
||||
[(i % 3, f"term-{i}", i) for i in range(20)],
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
steps = 0
|
||||
while db._fts_teardown_trash_step():
|
||||
steps += 1
|
||||
assert steps < 10
|
||||
|
||||
assert db._has_fts_trash(conn) is False
|
||||
assert conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE name = "
|
||||
"'fts_v22_trash_messages_fts_idx'"
|
||||
).fetchone() is None
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user