From 22de2de2347b20d1314f370d8455afb499124828 Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Tue, 18 Aug 2026 20:37:40 +0200 Subject: [PATCH] docs(db): correct why VACUUM fails against an open reader The comment said PerformMaintenance inherits a five-second timeout before throwing. It does inherit that timeout, but it is not what happens here: a VACUUM on a connection with a live reader fails instantly with 'cannot VACUUM - SQL statements in progress'. That is SQLITE_ERROR, not SQLITE_BUSY. Busy handling only covers contention between different connections, so no timeout applies and no retry helps. The practical difference matters for the error message the UI will show: the DELETE has already committed when it fires, so the rows are gone and only the compaction is missing -- 'deleted but not compacted', not 'failed'. And PerformMaintenance batches VACUUM, REINDEX and ANALYZE in one statement, so a failing VACUUM takes the other two with it. --- HellionChat/Util/DbOperationGate.cs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/HellionChat/Util/DbOperationGate.cs b/HellionChat/Util/DbOperationGate.cs index 7adc8c6..da720aa 100644 --- a/HellionChat/Util/DbOperationGate.cs +++ b/HellionChat/Util/DbOperationGate.cs @@ -14,12 +14,17 @@ internal enum DbOperation // frame. Generalises the retention-sweep lock, which already did exactly this // for a single case. // -// The reason it has to cover all of them together, not one each: an export -// leaves a reader open on the primary connection deliberately outside _readLock, -// because the enumerator is consumed lazily by its caller. If a VACUUM starts -// while that reader lives, it meets an active reader on a connection Microsoft -// documents as not thread-safe, and PerformMaintenance sets no command timeout -// so it inherits five seconds before throwing. +// The reason it has to cover all of them together, not one each: an export can +// leave a reader open on the primary connection outside _readLock, because +// StreamForExport hands back an enumerator its caller consumes lazily. A VACUUM +// starting while that reader lives fails immediately with "cannot VACUUM - SQL +// statements in progress". +// +// That is SQLITE_ERROR, not SQLITE_BUSY, so no timeout applies and no retry +// helps -- busy handling only covers contention between different connections. +// And it fails after the DELETE has committed, so the rows are gone and the file +// is not compacted. PerformMaintenance runs VACUUM, REINDEX and ANALYZE as one +// batch, so the latter two never run either. // // Pure state machine, no ImGui and no database, so the build suite can pin the // transitions without standing up either.