Restored fix 0ca9fe5e64 and added regression test. All 5 client-op tests pass. The test documents that gc-kvs-table! crashes on client-ops db (empty kvs, no schema at addr 0), which is why gc-sqlite-dbs! must exclude client-ops db.

Result: {"status":"keep","metric":0}
This commit is contained in:
Tienson Qin
2026-05-03 20:28:13 +08:00
parent 29f0cb1310
commit a8a7fdf512
3 changed files with 18 additions and 19 deletions

1
autoresearch.jsonl Normal file
View File

@@ -0,0 +1 @@
{"run":1,"commit":"29f0cb1","metric":0,"metrics":{},"status":"keep","description":"Reverted fix 0ca9fe5e64 and reproduced GC crash on client-ops db. gc-kvs-table! crashes with 'Expected first argument to be a string' because client-ops kvs table is empty (no Datascript schema at addr 0).","timestamp":1777811253591,"segment":0,"confidence":null}

View File

@@ -378,16 +378,15 @@
(defn- gc-sqlite-dbs!
"Gc main db weekly and rtc ops db each time when opening it"
[sqlite-db client-ops-db datascript-conn {:keys [full-gc?]}]
[sqlite-db datascript-conn {:keys [full-gc?]}]
(let [last-gc-at (:kv/value (d/entity @datascript-conn :logseq.kv/graph-last-gc-at))]
(when (or full-gc?
(nil? last-gc-at)
(not (number? last-gc-at))
(> (- (common-util/time-ms) last-gc-at) (* 30 24 3600 1000))) ; 1 month ago
(log/info :gc-sqlite-dbs "gc current graph")
(doseq [db (if @*publishing? [sqlite-db] [sqlite-db client-ops-db])]
(sqlite-gc/gc-kvs-table! db {:full-gc? full-gc?})
(.exec db "VACUUM"))
(sqlite-gc/gc-kvs-table! sqlite-db {:full-gc? full-gc?})
(.exec sqlite-db "VACUUM")
(ldb/transact! datascript-conn [{:db/ident :logseq.kv/graph-last-gc-at
:kv/value (common-util/time-ms)}]
{:skip-validate-db? true
@@ -480,7 +479,7 @@
{:initial-db? true})))]
(when-not sync-download-graph?
(db-migrate/migrate conn)
(gc-sqlite-dbs! db client-ops-db conn {})
(gc-sqlite-dbs! db conn {})
(maybe-run-recycle-gc! conn))
(when initial-tx-report
@@ -1220,10 +1219,10 @@
(def-thread-api :thread-api/gc-graph
[repo]
(let [{:keys [db client-ops]} (get @*sqlite-conns repo)
(let [{:keys [db]} (get @*sqlite-conns repo)
conn (get @*datascript-conns repo)]
(when (and db conn)
(gc-sqlite-dbs! db client-ops conn {:full-gc? true})
(gc-sqlite-dbs! db conn {:full-gc? true})
nil)))
(def-thread-api :thread-api/mobile-logs

View File

@@ -140,18 +140,17 @@
(is (= 42 (client-op/get-local-tx repo))
"local-tx should be 42 before GC")
;; GC on client-ops db: kvs table is empty.
;; With better-sqlite3 (Node.js), this is a no-op.
;; With WASM sqlite (browser), reading addr 0 from empty kvs can
;; return undefined/null in ways that crash transit parsing.
(testing "gc-kvs-table! on client-ops db with empty kvs table"
(is (try
(sqlite-gc/gc-kvs-table! db {:full-gc? false})
true
(catch :default e
(println :gc-kvs-table!-failed-on-client-ops (ex-message e))
false))
"gc-kvs-table! should not crash on client-ops db"))
;; GC on client-ops db: kvs table is empty because client-ops db
;; does not store Datascript data. gc-kvs-table! expects a valid
;; Datascript schema at kvs addr 0, which doesn't exist here.
;; This causes transit-js to throw "Expected first argument to be
;; a string" because it receives undefined instead of a transit
;; string. This is why gc-sqlite-dbs! must NOT include client-ops db.
(testing "gc-kvs-table! crashes on client-ops db with empty kvs table"
(is (thrown-with-msg?
js/Error
#"Expected first argument to be a string"
(sqlite-gc/gc-kvs-table! db {:full-gc? false}))))
;; After GC, local-tx should still be intact
(is (= 42 (client-op/get-local-tx repo))