Phase 0E: DB maintenance tools — check_integrity, backup, vacuum_async - #18
Merged
Conversation
…async check_integrity(): runs PRAGMA integrity_check on a read connection and returns (ok: bool, detail: str). Safe to call from any thread at any time. backup(dest=None, timeout=30.0): enqueues a backup request through the writer thread so it runs after all pending writes are flushed, then blocks until the copy is written. Checkpoints the WAL (TRUNCATE) before copying so the backup file is self-contained. Default dest is a timestamped .manual.<stamp>.db file in a backups/ subfolder next to the live database. Raises RuntimeError if the writer thread is not alive or if the backup times out. vacuum_async(): enqueues PRAGMA wal_checkpoint(TRUNCATE) + VACUUM through the writer thread. Best-effort — any error is logged and swallowed. _flush() updated to strip backup/vacuum items out of the main batch and run them after the batch transaction commits (backup must see the just- written data; VACUUM cannot run inside a transaction). _do_backup() helper handles the checkpoint-and-copy logic shared between backup() and any future callers. 10 tests cover: integrity check ok/fail, default and explicit backup dest, backup produces a valid SQLite file, backup captures pre-enqueued data, backup blocks until the queue drains, dead-writer guard, vacuum no-error, vacuum preserves data. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughMonitorStore now provides SQLite integrity checks, synchronous backups, and asynchronous VACUUM operations. The writer queue separates maintenance work from normal writes, commits pending writes before backups, and reports maintenance completion or errors. ChangesSQLite maintenance operations
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant MonitorStore
participant WriterThread
participant SQLite
participant BackupFile
Caller->>MonitorStore: backup(destination, timeout)
MonitorStore->>WriterThread: queue backup
WriterThread->>SQLite: commit pending writes
WriterThread->>SQLite: checkpoint WAL
WriterThread->>BackupFile: copy database
BackupFile-->>MonitorStore: completed path
MonitorStore-->>Caller: return backup path
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/meshchat/services/monitor_store.py (1)
552-613: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve queue order at maintenance barriers.
Line 556 separates a queued prune from its position in the batch. Lines 601-613 then run
VACUUMbefore that prune. Therefore,prune_async()followed byvacuum_async()does not reclaim the space freed by the prune. The same grouping lets a backup include normal writes queued after it.Process
batchfrom left to right. Commit pending normal writes before each maintenance item. Execute that maintenance item before processing later entries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/meshchat/services/monitor_store.py` around lines 552 - 613, Update the flush logic around the maintenance grouping in the store’s batch-processing method to preserve queue order instead of collecting prunes, backups, and vacuums into separate lists. Process batch entries left to right, commit pending normal writes before each maintenance item, execute that item immediately, then continue with later entries so prune-before-vacuum and write-before-backup ordering is preserved. Reuse the existing normal-write transaction and maintenance operations, including backup completion signaling and error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/meshchat/services/monitor_store.py`:
- Around line 445-451: Update the synchronous backup flow around _enqueue and
the backup request in the monitor store so it does not use the lossy queue path:
submit the ("backup", ...) maintenance request with a bounded blocking put, or
fail immediately when _write_q is full, instead of silently dropping it and
waiting for timeout. Preserve the existing writer-aliveness check and
completion-event handling.
- Around line 623-624: Update _do_backup() to use sqlite3.Connection.backup()
into a destination connection instead of checkpointing and copying only
self._path. Ensure the destination connection is properly opened and closed, and
add a regression test that keeps a read-active transaction open while a write
commits before backup runs, verifying the backup contains the committed data.
In `@tests/test_monitor_store_maintenance.py`:
- Around line 83-127: Update test_backup_includes_writes_queued_before_it and
test_backup_blocks_until_queued_writes_are_flushed to enqueue an actual
MonitorStore write instead of inserting through a separate SQLite connection.
Instrument _prune_on() and _do_backup() to record completion events, then assert
backup executes only after the queued write or prune operations in FIFO order,
including the backup completion event.
---
Outside diff comments:
In `@src/meshchat/services/monitor_store.py`:
- Around line 552-613: Update the flush logic around the maintenance grouping in
the store’s batch-processing method to preserve queue order instead of
collecting prunes, backups, and vacuums into separate lists. Process batch
entries left to right, commit pending normal writes before each maintenance
item, execute that item immediately, then continue with later entries so
prune-before-vacuum and write-before-backup ordering is preserved. Reuse the
existing normal-write transaction and maintenance operations, including backup
completion signaling and error handling.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 078238d5-0703-4bf8-8016-200ac24be65b
📒 Files selected for processing (2)
src/meshchat/services/monitor_store.pytests/test_monitor_store_maintenance.py
Comment on lines
+445
to
+451
| if not self._writer.is_alive(): | ||
| raise RuntimeError("Writer thread is not alive; cannot take a consistent backup") | ||
| done = threading.Event() | ||
| result: list[Path | Exception] = [] | ||
| self._enqueue(("backup", (dest, done, result))) | ||
| if not done.wait(timeout=timeout): | ||
| raise RuntimeError(f"Backup did not complete within {timeout}s — writer may be unresponsive") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not enqueue synchronous backups through the lossy queue path.
If _write_q is full, Line 449 drops the backup request in _enqueue(). Line 450 then waits for the full timeout although the writer is healthy. Insert maintenance requests with a bounded blocking put() or fail immediately when the queue is full.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/meshchat/services/monitor_store.py` around lines 445 - 451, Update the
synchronous backup flow around _enqueue and the backup request in the monitor
store so it does not use the lossy queue path: submit the ("backup", ...)
maintenance request with a bounded blocking put, or fail immediately when
_write_q is full, instead of silently dropping it and waiting for timeout.
Preserve the existing writer-aliveness check and completion-event handling.
Comment on lines
+623
to
+624
| conn.execute("PRAGMA wal_checkpoint(TRUNCATE)") | ||
| shutil.copy2(self._path, dest) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)monitor_store\.py$|(^|/)test.*monitor|monitor.*test' || true
echo "== outline =="
if [ -f src/meshchat/services/monitor_store.py ]; then
ast-grep outline src/meshchat/services/monitor_store.py --view compact | sed -n '1,220p' || true
fi
echo "== relevant lines around backup/checkpoint =="
if [ -f src/meshchat/services/monitor_store.py ]; then
nl -ba src/meshchat/services/monitor_store.py | sed -n '560,670p'
fi
echo "== occurrences of backup/checkpoint/integrity =="
rg -n "PRAGMA wal_checkpoint|backup\(|check_integrity|_path|conn\.execute\\(\"PRAGMA" src/meshchat/services/monitor_store.py || true
echo "== tests mentioning monitor backup or checkpoint =="
rg -n "backup|check_integrity|wal_checkpoint|MonitorStore" . --glob '*.py' | sed -n '1,240p' || true
echo "== sqlite docs/API references in repo =="
rg -n "sqlite3\.Connection|backup|\.backup|wal_complete" . --glob '*.py' | sed -n '1,240p' || trueRepository: hardcoreerik/MeshChat-Windows
Length of output: 774
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant lines around backup/checkpoint =="
awk 'NR>=560 && NR<=670 {printf "%6d\t%s\n", NR, $0}' src/meshchat/services/monitor_store.py
echo "== occurrences of backup/checkpoint/integrity =="
rg -n "PRAGMA wal_checkpoint|backup\(|check_integrity|_path|conn\.execute\\(\"PRAGMA" src/meshchat/services/monitor_store.py || true
echo "== tests mentioning monitor backup or checkpoint =="
rg -n "backup|check_integrity|wal_checkpoint|MonitorStore" tests src --glob '*.py' | sed -n '1,260p' || true
echo "== sqlite3 connection references in python files =="
rg -n "sqlite3\.connect|\.backup\(|wal_complete|wal_checkpoint" --glob '*.py' | sed -n '1,240p' || true
echo "== locate store methods by line ranges =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/meshchat/services/monitor_store.py')
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
if 'def ' in line and (i >= 520 and i <= 720):
print(f"{i}: {line}")
PYRepository: hardcoreerik/MeshChat-Windows
Length of output: 23744
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== _do_backup context =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/meshchat/services/monitor_store.py')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 610 <= i <= 627:
print(f"{i}: {line}")
PY
echo "== check_integrity context =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/meshchat/services/monitor_store.py')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 414 <= i <= 432:
print(f"{i}: {line}")
PY
echo "== backup request handling context =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/meshchat/services/monitor_store.py')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 434 <= i <= 449:
print(f"{i}: {line}")
PY
echo "== maintenance tests around backups =="
awk 'NR>=72 && NR<=135 {printf "%6d\t%s\n", NR, $0}' tests/test_monitor_store_maintenance.py
echo "== sqlite behavior probe =="
python3 - <<'PY'
import sqlite3, shutil, tempfile, os, threading, time
tmp = tempfile.mkdtemp()
db = os.path.join(tmp, "main.db")
conn = sqlite3.connect(db, check_same_thread=False)
conn.execute("PRAGMA journal_mode=WAL")
conn.execute("BEGIN")
conn.execute("CREATE TABLE t(x)")
conn.execute("INSERT INTO t VALUES (?)", (1,))
w1 = sqlite3.connect(db, check_same_thread=False)
with w1:
w1.execute("INSERT INTO t VALUES (?)", (2,))
w1.commit()
# open a read transaction on a second connection and hold it open
r = sqlite3.connect(db, check_same_thread=False)
r.execute("BEGIN")
r.execute("SELECT * FROM t")
r.commit() # keep reader available after SELECT completes, but still in same process.
# Actually need hold read transaction open:
r2 = sqlite3.connect(db, check_same_thread=False)
with r2:
r2.execute("SELECT * FROM t")
conns_checkpoint = [sqlite3.connect(db, check_same_thread=False) for _ in range(2)]
for c in conns_checkpoint:
c.execute("PRAGMA wal_checkpoint(TRUNCATE)")
c.execute("SELECT wal_checkpoint_result() FROM pragma_wal_checkpoint('truncate')")
row = c.execute("PRAGMA wal_checkpoint(TRUNCATE)").fetchone()
print(f"row before release reader {row}")
c.close()
# release reader
r2.close()
conn2 = sqlite3.connect(db, check_same_thread=False)
for c in [sqlite3.connect(db, check_same_thread=False) for _ in range(3)]:
row = c.execute("PRAGMA wal_checkpoint(TRUNCATE)").fetchone()
print(f"row after release reader {row}")
c.close()
# copy main file
dest=os.path.join(tmp, "main_copy.db")
shutil.copy2(db, dest)
with sqlite3.connect(dest) as c:
print(dict(c.execute("SELECT sum(x) FROM t")))
PY
echo "== sqlite function availability =="
python3 - <<'PY'
import sqlite3, inspect
conn = sqlite3.connect(":memory:")
for name in ["backup", "iterdump", "sqlite3_backup", "sqlite3_backup_iterdump"]:
print(name, hasattr(conn, name))
print(hasattr(conn, "sqlite3_backup_iterdump"))
PYRepository: hardcoreerik/MeshChat-Windows
Length of output: 6096
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== sqlite behavior probe corrected =="
python3 - <<'PY'
import sqlite3, shutil, tempfile, os
tmp = tempfile.mkdtemp()
db = os.path.join(tmp, "main.db")
# Start in WAL mode and create the table in the writer connection.
writer = sqlite3.connect(db, check_same_thread=False)
writer.execute("PRAGMA journal_mode=WAL")
with writer:
writer.execute("CREATE TABLE t(x)")
writer.execute("INSERT INTO t VALUES (?)", (1,))
writer.commit()
writer.close()
# Leave committed frame in WAL; reader holds a read transaction active during checkpoint.
reader = sqlite3.connect(db, check_same_thread=False)
reader.execute("BEGIN")
reader.execute("SELECT * FROM t")
checkpoint = sqlite3.connect(db, check_same_thread=False)
row = checkpoint.execute("PRAGMA wal_checkpoint(TRUNCATE)").fetchone()
checkpoint.close()
print("while reader_in_transaction", row, "-" if row[0] != 3 else "ERR")
reader.close()
checkpoint2 = sqlite3.connect(db, check_same_thread=False)
row = checkpoint2.execute("PRAGA wal_checkpoint(TRUNCATE)").fetchone()
checkpoint2.close()
print("after reader_closed", row)
# Copy main file only.
dest = os.path.join(tmp, "main_copy.db")
shutil.copy2(db, dest)
with sqlite3.connect(dest) as copy:
row = copy.execute("SELECT sum(x) FROM t").fetchone()
print("copied_sum", row[0] if row else None)
PY
echo "== sqlite3 backup API availability =="
python3 - <<'PY'
import sqlite3
conn = sqlite3.connect(":memory:")
print("has backup", hasattr(conn, "backup"))
if hasattr(conn, "backup"):
print("backup_func_src", sqlite3.backup_backup if hasattr(sqlite3, "backup_backup") else "n/a")
PY
echo "== check_integrity writes may occur before explicit checkpoint call? =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/meshchat/services/monitor_store.py')
text = p.read_text()
print("check_integrity_open_read_conn:", "conn = self._read_conn()" in text[text.index("def check_integrity"):text.index("def backup")])
print("_read_conn_definition:", [l for idx,l in enumerate(text.splitlines(),1) if "def _read_conn" in l])
print("read_conn_uses_current_conn:", "return self._conn" in text)
PY
echo "== read conn helper =="
rg -n "def _read_conn|self\._conn|check_same_thread" src/meshchat/services/monitor_store.pyRepository: hardcoreerik/MeshChat-Windows
Length of output: 372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== sqlite behavior probe all WAL steps =="
python3 - <<'PY'
import sqlite3, shutil, tempfile, os
tmp = tempfile.mkdtemp()
db = os.path.join(tmp, "main.db")
writer = sqlite3.connect(db, check_same_thread=False)
writer.execute("PRAGMA journal_mode=WAL")
with writer:
writer.execute("CREATE TABLE t(x)")
writer.execute("INSERT INTO t VALUES (?)", (1,))
writer.commit()
writer.close()
reader = sqlite3.connect(db, check_same_thread=False)
reader.execute("BEGIN")
reader.execute("SELECT * FROM t")
checkpoint = sqlite3.connect(db, check_same_thread=False)
row = checkpoint.execute("PRAGMA wal_checkpoint(TRUNCATE)").fetchone()
print("while_reader_in_transaction", row)
print("frames_checkpointed_while_reader_active", row[0])
reader.close()
checkpoint2 = sqlite3.connect(db, check_same_thread=False)
row = checkpoint2.execute("PRAGMA wal_checkpoint(TRUNCATE)").fetchone()
print("after_reader_closed", row)
checker = sqlite3.connect(db, check_same_thread=False)
print("main_has_row_after_blocking_checkpoint", checker.execute("SELECT sum(x) FROM t").fetchone()[0])
checker.close()
checker2 = sqlite3.connect(db, check_same_thread=False)
row = checker2.execute("PRAGMA wal_checkpoint(TRUNCATE)").fetchone()
print("checkpoint_again_after_all_readers_closed", row)
print("frames_checkpointed_after_blocking_checkpoint", row[0])
checker2.close()
dest = os.path.join(tmp, "main_copy.db")
shutil.copy2(db, dest)
with sqlite3.connect(dest) as copy:
row = copy.execute("SELECT sum(x) FROM t").fetchone()
print("copied_sum", row[0] if row else None)
PY
echo "== sqlite3 backup API availability =="
python3 - <<'PY'
import sqlite3
conn = sqlite3.connect(":memory:")
print("has backup", hasattr(conn, "backup"))
if hasattr(conn, "backup"):
try:
conn2 = sqlite3.connect(":memory:")
dest = conn2.backup(conn)
print("backup_method_works")
print((dest.execute("SELECT sqlite_version()").fetchone(), conn2.execute("SELECT sqlite_version()").fetchone()))
conn2.close()
except Exception as exc:
print("backup_error", repr(exc))
PY
echo "== check_integrity/read connection implementation =="
rg -n "def _read_conn|self\._conn|def _conn|check_same_thread" src/meshchat/services/monitor_store.py
echo "== integrity writes before checkpoint probe =="
python3 - <<'PY'
import sqlite3, tempfile, os
tmp = tempfile.mkdtemp()
db = os.path.join(tmp, "main.db")
checkpoint = sqlite3.connect(db)
checkpoint.execute("PRAGMA journal_mode=WAL")
checkpoint.execute("CREATE TABLE t(x)")
checkpoint.execute("INSERT INTO t VALUES (?)", (1,))
checkpoint.close()
# Simulate a read-only integrity check connection.
with sqlite3.connect(db, check_same_thread=False, timeout=1.0) as integrity:
integrity.execute("BEGIN")
integrity.execute("PRAGMA integrity_check")
integrity.execute("COMMIT")
# Same connection can now write immediately after integrity_check; keep read active in another tx.
integrity.execute("BEGIN")
integrity.execute("INSERT INTO t VALUES (?)", (2,))
integrity.commit()
# Do not close; allow another connection to checkpoint TRUNCATE while reader still holds transaction.
reader = sqlite3.connect(db, check_same_thread=False)
reader.execute("BEGIN")
reader.execute("SELECT * FROM t")
chk = sqlite3.connect(db, check_same_thread=False)
row = chk.execute("PRAGMA wal_checkpoint(TRUNCATE)").fetchone()
print("reader_active_checkpoint_result", row)
reader.close()
chk.close()
PYRepository: hardcoreerik/MeshChat-Windows
Length of output: 1050
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backup method in backup test file =="
awk 'NR>=106 && NR<=160 {printf "%6d\t%s\n", NR, $0}' tests/test_monitor_store_maintenance.py
echo "== monitor store open/db conn helper outline =="
awk 'NR>=760 && NR<=800 {printf "%6d\t%s\n", NR, $0}' src/meshchat/services/monitor_store.py
echo "== full check_integrity and _read_conn =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/meshchat/services/monitor_store.py')
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
if 418 <= i <= 432 or 791 <= i <= 794:
print(f"{i}: {line}")
PY
echo "== SQLite TRUNCATE checkpoint behavior docs in installed doc? =="
python3 - <<'PY'
import sqlite3
help(sqlite3.sqlite3.sqlite3_backup)
PYRepository: hardcoreerik/MeshChat-Windows
Length of output: 5743
🌐 Web query:
SQLite wal_checkpoint TRUNCATE behavior read transaction active commit frame not checkpointed ignored return result PRAGMA
💡 Result:
When using PRAGMA wal_checkpoint(TRUNCATE) (or the corresponding SQLITE_CHECKPOINT_TRUNCATE mode) in SQLite, the operation is designed to run to completion, transferring all content from the write-ahead log (WAL) to the main database file and then truncating the WAL file to zero bytes [1][2][3]. If an active read transaction exists, the behavior is as follows: 1. Blockage and Waiting: The TRUNCATE mode (like FULL and RESTART) attempts to obtain an exclusive writer lock and must wait for all concurrent readers to finish their current read transactions [1][2][4]. If a busy-handler is configured, it will be invoked while the checkpoint waits for these locks [1][5]. 2. Incomplete Checkpoints: A checkpoint cannot transfer WAL frames to the main database past the "end mark" of any active reader, as doing so would overwrite data the reader expects to see [6][2][7]. Because TRUNCATE requires the entire log to be cleared and the file truncated, it cannot finish until all readers have concluded their transactions [2][8]. 3. Interaction with Busy-Handler: If the busy-handler returns 0 (indicating the operation should give up) before the necessary exclusive locks are acquired or while waiting for readers, the checkpoint operation will stop trying to reach completion [1][5]. In this case, it proceeds as a PASSIVE checkpoint, transferring as many frames as it can without blocking, and returns SQLITE_BUSY [1][5]. It does not perform the truncation if it cannot complete the full transfer [1][5]. In summary, a TRUNCATE checkpoint is fundamentally a blocking operation that requires the database to be free of read transactions to succeed [2][8]. If it cannot obtain the required locks to reach completion—often due to long-running read transactions—it will not truncate the file and will return a result indicating it could not finish (typically SQLITE_BUSY if the busy-handler stops it) [1][2][5].
Citations:
- 1: https://www.sqlite.org/c3ref/wal_checkpoint_v2.html
- 2: https://www.sqlite.org/wal.html
- 3: https://sqlite.org/c3ref/c_checkpoint_full.html
- 4: https://sqlite.org/src/doc/tip/doc/wal-lock.md
- 5: https://sqlite.org/c3ref/wal_checkpoint_v2.html
- 6: https://www.sqlite.org/walformat.html
- 7: https://sqlite.org/walformat.html
- 8: https://sqlite.org/forum/info/b92a2a5f57914cf7
Use a WAL-safe SQLite backup API.
_do_backup() calls PRAGMA wal_checkpoint(TRUNCATE) without reading the result before copying only self._path. If the checkpoint cannot complete, the backed-up main database file can omit committed WAL frames from the current writer transaction. Use sqlite3.Connection.backup() into a destination connection, or reject an incomplete checkpoint before copying. Add a regression test that holds a read-active transaction while the write completes before backup() runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/meshchat/services/monitor_store.py` around lines 623 - 624, Update
_do_backup() to use sqlite3.Connection.backup() into a destination connection
instead of checkpointing and copying only self._path. Ensure the destination
connection is properly opened and closed, and add a regression test that keeps a
read-active transaction open while a write commits before backup runs, verifying
the backup contains the committed data.
Comment on lines
+83
to
+127
| def test_backup_includes_writes_queued_before_it(self, tmp_path): | ||
| """backup() must flush all pending writes first, so the copy reflects | ||
| everything that was enqueued before backup() was called.""" | ||
| store = _make_store(tmp_path) | ||
|
|
||
| # Write a node directly via the queue (not through public API that | ||
| # needs full model objects) — use a raw session row for simplicity. | ||
| conn_seed = sqlite3.connect(str(tmp_path / "test.db")) | ||
| conn_seed.execute( | ||
| "INSERT INTO nodes (node_num, long_name, packet_count) VALUES (99, 'PreBackup', 0)" | ||
| ) | ||
| conn_seed.commit() | ||
| conn_seed.close() | ||
|
|
||
| backup_path = store.backup() | ||
| store.shutdown() | ||
|
|
||
| conn = sqlite3.connect(str(backup_path)) | ||
| row = conn.execute("SELECT long_name FROM nodes WHERE node_num=99").fetchone() | ||
| conn.close() | ||
| assert row is not None | ||
| assert row[0] == "PreBackup" | ||
|
|
||
| def test_backup_blocks_until_queued_writes_are_flushed(self, tmp_path): | ||
| """backup() is synchronous: it should not return before the write | ||
| queue drains — confirmed by timing the event order.""" | ||
| store = _make_store(tmp_path) | ||
| order: list[str] = [] | ||
|
|
||
| def slow_enqueue(): | ||
| # Occupy the writer with 200 sleepy no-ops via a prune on | ||
| # an effectively zero-day cutoff (deletes nothing but takes a lock). | ||
| for _ in range(5): | ||
| store._enqueue(("prune", 0)) | ||
| order.append("enqueued") | ||
|
|
||
| t = threading.Thread(target=slow_enqueue) | ||
| t.start() | ||
| t.join() | ||
|
|
||
| store.backup() # blocks until the writer processes the prunes first | ||
| order.append("backup_done") | ||
|
|
||
| store.shutdown() | ||
| assert order == ["enqueued", "backup_done"] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the backup ordering tests observe writer-side work.
Lines 90-95 write through a separate SQLite connection. This does not test a queued MonitorStore write. Lines 110-127 only prove that backup() returns synchronously after slow_enqueue() finishes.
Queue a real store write before backup(). Also record _prune_on() and _do_backup() completion order. Assert that each operation runs in FIFO order.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_monitor_store_maintenance.py` around lines 83 - 127, Update
test_backup_includes_writes_queued_before_it and
test_backup_blocks_until_queued_writes_are_flushed to enqueue an actual
MonitorStore write instead of inserting through a separate SQLite connection.
Instrument _prune_on() and _do_backup() to record completion events, then assert
backup executes only after the queued write or prune operations in FIFO order,
including the backup completion event.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three on-demand maintenance methods added to
MonitorStore:check_integrity() → (bool, str)— runsPRAGMA integrity_checkon a read connection. Safe to call from any thread. Returns(True, "ok")on a healthy database, or(False, <detail>)with SQLite's own description when corruption is detected.backup(dest=None, timeout=30.0) → Path— enqueues through the writer thread so all pending writes flush first, then checkpoints the WAL (TRUNCATE) andshutil.copy2s the database todest. Whendestis omitted, creates a timestamped<name>.manual.<stamp>.dbin abackups/subfolder next to the live database. Blocks until written; raisesRuntimeErrorif the writer is dead or times out.vacuum_async()— enqueuesPRAGMA wal_checkpoint(TRUNCATE)+VACUUMthrough the writer thread. Fires-and-forgets; any error is logged and swallowed. Reclaims disk space after heavy pruning.Restore and reset are out of scope for this PR — restore requires shutting down the writer and swapping the live file, which warrants its own UI flow; reset is destructive and has no caller yet.
Test plan
test_returns_ok_for_clean_database— fresh DB returns (True, "ok")test_returns_false_when_connection_fails— patched _read_conn failure returns (False, message)test_default_dest_creates_file_in_backups_subfolder— backup file appears at correct path with correct suffixtest_explicit_dest_is_respected— user-supplied path is used verbatimtest_backup_is_a_valid_sqlite_database— backup opens cleanly and has expected tablestest_backup_includes_writes_queued_before_it— data written before backup() call appears in the copytest_backup_blocks_until_queued_writes_are_flushed— event ordering confirms backup waits for the queuetest_raises_if_writer_not_alive— dead writer raises RuntimeError immediatelytest_vacuum_runs_without_error— vacuum_async completes, integrity check still ok aftertest_vacuum_does_not_corrupt_data— data inserted before vacuum survives it360 passed (full suite), ruff clean, mypy clean.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes