Fix web UI slowness: delete drain starved the app, janitor race stranded tombstones

openmind/db.py

@@ -397,6 +397,20 @@ def delete_file_index_entry(project_id: str, file_path: str) -> None:
         _c().commit()
 
 
+def count_file_index(project_id: str) -> int:
+    """How many files are indexed, WITHOUT materializing them.
+
+    ``get_file_index`` builds a dict of every row and json-decodes two columns
+    per row; callers that only wanted ``len()`` of it (the project header card)
+    paid ~36ms on a 6.5k-file project to learn a number SQLite answers in under
+    a millisecond off the (project_id, file_path) primary-key index."""
+    with _lock:
+        row = _c().execute(
+            "SELECT COUNT(*) FROM file_index WHERE project_id=?", (project_id,)
+        ).fetchone()
+    return int(row[0]) if row else 0
+
+
 def clear_file_index(project_id: str) -> None:
     with _lock:
         _c().execute("DELETE FROM file_index WHERE project_id=?", (project_id,))

openmind/jobs.py

@@ -51,6 +51,12 @@ def start_worker() -> None:
     t.start()
 
 
+def shutdown_requested() -> bool:
+    """Public predicate for the shutdown flag, so other modules can pass a
+    ``cancel`` callback into long drains without importing a private Event."""
+    return _shutdown.is_set()
+
+
 def begin_shutdown() -> None:
     """Called from the app lifespan on shutdown. Interrupts any background
     delete-cleanup at its next batch (the 'deleting' tombstone makes it resume
@@ -408,8 +414,11 @@ def terminate_project(project_id: str, clear_cases: bool = False) -> Dict[str, A
     _stop_project_jobs(project_id)
 
     # 1) drop the code collection (delete-only; the next learn recreates it lazily —
-    # recreating an empty collection now is wasted work on a large project)
-    vectorstore.get_code_store(project_id).drop()
+    # recreating an empty collection now is wasted work on a large project).
+    # cancel= matters even though this one is synchronous: without it a Ctrl+C
+    # during a large Terminate waits out the entire drain, which is exactly the
+    # dead-Ctrl+C symptom the batched drain was introduced to remove.
+    vectorstore.get_code_store(project_id).drop(cancel=_shutdown.is_set)
     # 2) delete map/* and docs/*
     for sub in ("map", "docs"):
         d = config.project_dir(project_id) / sub

openmind/main.py

@@ -72,7 +72,19 @@ def _warm_vectorstore() -> None:
     see sweep_orphan_segment_dirs for why it cannot run here.)"""
     try:
         vectorstore.backend_name()
-        orphans = vectorstore.drop_orphan_collections({p["id"] for p in db.list_projects()})
+        # Same union as _sweep_orphan_project_dirs above, for the same reason: a
+        # 'deleting' project's collections are NOT orphans — a cleanup thread
+        # already owns them (jobs._recover_on_restart spawned it moments ago in
+        # lifespan). db.list_projects() with no argument EXCLUDES 'deleting', so
+        # passing it alone made the janitor race that cleanup: whichever thread
+        # reached delete_collection first made the other's next get() raise, the
+        # cleanup read that as "interrupted" and returned BEFORE removing the
+        # data dir and the project row — so the tombstone came back every boot
+        # and the project could never finish deleting.
+        known = {p["id"] for p in db.list_projects()} | \
+                {p["id"] for p in db.list_projects("deleting")}
+        orphans = vectorstore.drop_orphan_collections(
+            known, cancel=jobs.shutdown_requested)
         if orphans:
             print(f"[startup] dropped {len(orphans)} orphaned vector collection(s): "
                   f"{', '.join(orphans)}", flush=True)
@@ -103,6 +115,16 @@ async def _lifespan(app: FastAPI):
     if swept:
         print(f"[startup] removed {len(swept)} leaked vector segment dir(s).",
               flush=True)
+    # Say out loud that storage reclaim is pending. A tombstone whose cleanup
+    # keeps failing used to be completely silent: the project simply vanished
+    # from the UI while every boot quietly re-ran a multi-minute drain, so the
+    # app felt slow for reasons nothing on screen could explain.
+    pending = db.list_projects("deleting")
+    if pending:
+        print(f"[startup] {len(pending)} project(s) pending storage reclaim: "
+              f"{', '.join(p['id'] for p in pending)}. The app stays usable "
+              f"while this runs; watch '[vectorstore] dropping ...' for progress.",
+              flush=True)
     # The worker is opt-in per surface; the web app always needs one.
     runtime.ensure_worker()
     threading.Thread(target=_warm_vectorstore, name="vs-warmup", daemon=True).start()

openmind/ports/workspace_repository.py

@@ -38,5 +38,7 @@ def remove_project_path(self, project_id: str, path: str) -> None: ...
 
     def get_file_index(self, project_id: str) -> Dict[str, Dict[str, Any]]: ...
 
+    def count_file_index(self, project_id: str) -> int: ...
+
 
 __all__ = ["WorkspaceRepository"]

openmind/services/workspace_service.py

@@ -61,11 +61,19 @@ def exists(self, workspace_id: str) -> bool:
 
     def describe(self, workspace_id: str) -> Dict[str, Any]:
         """The workspace record decorated with live store counts — the shape
-        ``GET /projects/{id}`` returns."""
+        ``GET /projects/{id}`` returns.
+
+        These three numbers are header-card decoration, so they are gathered the
+        cheapest way that is still correct: counts come from collections that
+        already exist (never ``get_or_create``, which would create storage from a
+        read), and the file count is a ``COUNT(*)`` rather than a full row load.
+        """
         record = self.get(workspace_id)
-        record["code_chunks"] = vectorstore.get_code_store(workspace_id).count()
-        record["cases_count"] = vectorstore.get_cases_store(workspace_id).count()
-        record["files_indexed"] = len(self._repo.get_file_index(workspace_id))
+        record["code_chunks"] = vectorstore.count_collection(
+            vectorstore.code_collection_name(workspace_id))
+        record["cases_count"] = vectorstore.count_collection(
+            vectorstore.cases_collection_name(workspace_id))
+        record["files_indexed"] = self._repo.count_file_index(workspace_id)
         return record
 
     # -- creation -----------------------------------------------------------
@@ -167,11 +175,13 @@ def status(self, workspace_id: str) -> Dict[str, Any]:
             "template": templates.selection_info(record),
             "counts": {
                 "indexed_files": _count(
-                    lambda: len(self._repo.get_file_index(workspace_id))),
+                    lambda: self._repo.count_file_index(workspace_id)),
                 "code_chunks": _count(
-                    lambda: vectorstore.get_code_store(workspace_id).count()),
+                    lambda: vectorstore.count_collection(
+                        vectorstore.code_collection_name(workspace_id))),
                 "solved_cases": _count(
-                    lambda: vectorstore.get_cases_store(workspace_id).count()),
+                    lambda: vectorstore.count_collection(
+                        vectorstore.cases_collection_name(workspace_id))),
                 "glossary_terms": _count(
                     lambda: len(mapio.load_glossary(workspace_id).get("terms") or {})),
             },

openmind/vectorstore.py

@@ -26,10 +26,52 @@
 _backend: Optional[str] = None
 _chroma_client = None
 
-# Rows drained per delete batch while dropping a collection (see
-# _drop_chroma_collection): ~0.5s of GIL hold per batch measured on a real
-# 2 GB store, small enough that the app stays responsive throughout.
-DROP_BATCH = 500
+# How the drain stays out of the app's way. BOTH knobs are load-bearing and they
+# control DIFFERENT things — see the measurements below before touching either.
+#
+#   DROP_BATCH        bounds the WORST-CASE stall (one rust call = one GIL hold)
+#   DROP_YIELD_RATIO  bounds the DUTY CYCLE (what share of the interpreter the
+#                     rest of the process gets while a drain runs)
+#
+# Measured on a copy of a real 2.1 GB store (79,585-chunk collection), with a
+# 10ms-tick thread standing in for the FastAPI threadpool:
+#
+#   batch  yield        drain      p95 stall   max stall   app availability
+#     500  10ms fixed    131s        823ms       1480ms          5%   <- shipped before
+#      50  10ms fixed    271s        187ms        352ms         23%
+#     500  0.6x hold     131s          2ms        768ms         39%
+#     200  0.5x hold     157s        170ms        359ms         38%   <- shipped now
+#
+# The old fixed 10ms gap was the bug: against a ~500ms GIL hold it left every
+# other Python thread on ~5% of the interpreter for the whole drain. That is why
+# "load a project" and "delete a project" felt frozen even though neither of
+# those code paths is itself slow (GET /projects/{id} measures 49-273ms idle).
+#
+# Note the trade in the table: a bigger batch buys throughput and a lower p95 but
+# a much worse TAIL, and it is the tail a user actually feels as a freeze. 200 is
+# chosen for the 359ms worst case, at ~20% more drain wall-clock than 500 — the
+# drain is background work, so wall-clock is the cheapest thing to spend here.
+DROP_BATCH = 200
+DROP_YIELD_RATIO = 0.5
+DROP_YIELD_MAX = 1.0
+
+# Collection names with a drain in flight IN THIS PROCESS. Two threads draining
+# one collection is not merely wasteful: whichever reaches delete_collection
+# first makes the other's next get() raise, and that exception used to be
+# reported as "interrupted" — abandoning a cleanup that had not yet removed the
+# data dir or the project row. See _drop_chroma_collection.
+_dropping_lock = threading.Lock()
+_dropping: set = set()
+
+
+def _is_missing_collection(exc: BaseException) -> bool:
+    """True when *exc* means "that collection isn't there" rather than a real
+    failure. Matched by name/text because the exception type moved between
+    chromadb releases (NotFoundError / InvalidCollectionException / ValueError)."""
+    if type(exc).__name__ in ("NotFoundError", "InvalidCollectionException"):
+        return True
+    text = str(exc).lower()
+    return "does not exist" in text or "not found" in text
 
 
 def _detect_backend() -> str:
@@ -78,36 +120,68 @@ def _drop_chroma_collection(name: str, cancel: Optional[Callable[[], bool]] = No
     the caller's 'deleting' tombstone re-spawns the cleanup on the next start
     and the drain RESUMES where it stopped.
 
+    Batching bounds a SINGLE stall; it is the gap between batches that keeps the
+    app alive, and that gap is sized from each batch's own measured hold (see
+    DROP_YIELD_RATIO). The original fixed 10ms gap did not — it left the rest of
+    the process on ~5% of the interpreter for the whole drain, which is what made
+    "load a project" and "delete a project" feel frozen even though neither of
+    those code paths was itself slow.
+
     Returns True once the collection is gone (or never existed), False if
     *cancel* stopped the drain early or a batch failed (caller retries later).
     """
+    # One drainer per collection. A second concurrent drain of the same name
+    # cannot help (they fight over the same rows) and actively HURT: it used to
+    # make the owning cleanup abandon itself mid-way, stranding the project as a
+    # permanent 'deleting' tombstone.
+    with _dropping_lock:
+        if name in _dropping:
+            print(f"[vectorstore] drop of {name} already in flight; skipping "
+                  f"duplicate request.", flush=True)
+            return False
+        _dropping.add(name)
     try:
-        col = _chroma_client.get_collection(name)
-    except Exception:
-        return True   # no such collection — nothing to drop
-    try:
-        total = col.count()
-        drained = 0
-        while True:
-            if cancel is not None and cancel():
-                print(f"[vectorstore] drop of {name} paused at "
-                      f"{drained}/{total} (shutdown); resumes on next start.",
-                      flush=True)
-                return False
-            ids = col.get(limit=DROP_BATCH, include=[]).get("ids") or []
-            if not ids:
-                break
-            col.delete(ids=ids)
-            drained += len(ids)
-            if drained % 10_000 < DROP_BATCH:
-                print(f"[vectorstore] dropping {name}: {drained}/{total}", flush=True)
-            time.sleep(0.01)   # deliberate GIL gap: let requests + signals run
-        _chroma_client.delete_collection(name)
-        return True
-    except Exception as exc:
-        print(f"[vectorstore] drop of {name} interrupted: {exc!r} "
-              f"(cleanup will retry on the next start)", flush=True)
-        return False
+        try:
+            col = _chroma_client.get_collection(name)
+        except Exception:
+            return True   # no such collection — nothing to drop
+        try:
+            total = col.count()
+            drained = 0
+            while True:
+                if cancel is not None and cancel():
+                    print(f"[vectorstore] drop of {name} paused at "
+                          f"{drained}/{total} (shutdown); resumes on next start.",
+                          flush=True)
+                    return False
+                started = time.monotonic()
+                ids = col.get(limit=DROP_BATCH, include=[]).get("ids") or []
+                if not ids:
+                    break
+                col.delete(ids=ids)
+                drained += len(ids)
+                if drained % 10_000 < DROP_BATCH:
+                    print(f"[vectorstore] dropping {name}: {drained}/{total}",
+                          flush=True)
+                # Deliberate GIL gap, sized from the hold we just took so the
+                # rest of the process keeps running (see DROP_YIELD_RATIO).
+                held = time.monotonic() - started
+                time.sleep(min(DROP_YIELD_MAX, max(0.01, held * DROP_YIELD_RATIO)))
+            _chroma_client.delete_collection(name)
+            return True
+        except Exception as exc:
+            # "already gone" is SUCCESS, not interruption. Reporting it as a
+            # failure is what let a losing racer abandon a cleanup that still
+            # had the data dir and the project row to remove.
+            if _is_missing_collection(exc):
+                print(f"[vectorstore] drop of {name}: already gone.", flush=True)
+                return True
+            print(f"[vectorstore] drop of {name} interrupted: {exc!r} "
+                  f"(cleanup will retry on the next start)", flush=True)
+            return False
+    finally:
+        with _dropping_lock:
+            _dropping.discard(name)
 
 
 def drop_collection(collection_name: str,
@@ -460,6 +534,24 @@ def cases_collection_name(project_id: str) -> str:
     return f"cases_{project_id}"
 
 
+def count_collection(collection_name: str) -> int:
+    """Row count for a collection that may not exist yet — 0 if it doesn't.
+
+    Deliberately NOT ``get_store(...).count()``: that goes through
+    ``get_or_create_collection``, so merely LOOKING at a project created its
+    ``cases_<pid>`` collection as a side effect of a read-only page view (which
+    is why stores hold empty cases collections for projects that never solved a
+    case). Read paths should not create storage."""
+    if _detect_backend() != "chroma":
+        return NumpyStore(collection_name).count()
+    try:
+        return int(_chroma_client.get_collection(collection_name).count())
+    except Exception as exc:
+        if _is_missing_collection(exc):
+            return 0
+        raise
+
+
 def get_code_store(project_id: str) -> _Store:
     return get_store(code_collection_name(project_id))
 
@@ -475,13 +567,22 @@ def _pid_of(collection_name: str) -> Optional[str]:
     return None
 
 
-def drop_orphan_collections(known_project_ids) -> List[str]:
+def drop_orphan_collections(known_project_ids,
+                            cancel: Optional[Callable[[], bool]] = None) -> List[str]:
     """Delete ``code_<pid>`` / ``cases_<pid>`` collections whose project id is no
     longer in *known_project_ids* — storage leaked by older delete paths that
     reset-and-recreated collections instead of dropping them, and by interrupted
     deletes. Best-effort; returns the names dropped. Call at startup (no concurrent
     project creation) so a half-created project's collection is never mistaken for
-    an orphan."""
+    an orphan.
+
+    *known_project_ids* MUST include projects in the 'deleting' state. Their
+    collections still exist but are OWNED by a running cleanup thread, so
+    treating them as orphans starts a second, competing drain (see
+    _drop_chroma_collection's in-flight guard, which is the backstop for that).
+
+    Pass *cancel* so a shutdown interrupts the sweep at the next batch — without
+    it, Ctrl+C during a startup sweep waits out the whole drain."""
     known = set(known_project_ids)
     dropped: List[str] = []
     if _detect_backend() == "chroma":
@@ -490,12 +591,14 @@ def drop_orphan_collections(known_project_ids) -> List[str]:
         except Exception:
             return dropped
         for name in names:
+            if cancel is not None and cancel():
+                break
             pid = _pid_of(name)
             if pid and pid not in known:
                 # drained drop (never a one-shot delete_collection): an orphan
                 # can be a full-size learned collection, and this runs at
                 # startup where a GIL-held freeze would look like a hung boot.
-                if _drop_chroma_collection(name):
+                if _drop_chroma_collection(name, cancel):
                     dropped.append(name)
     else:
         npdir = config.CHROMA_DIR / "np"

scripts/run_acceptance.py

@@ -82,6 +82,8 @@ def path(self) -> Path:
     Script("verify_ask", CORE, "Ask gating when no model is ready"),
     Script("verify_ask2", CORE, "Ask history, cancellation and case saving"),
     Script("verify_async_delete", CORE, "asynchronous project delete"),
+    Script("verify_delete_race", CORE,
+           "delete completes: janitor/cleanup race, in-flight drain guard"),
     Script("verify_source_link", CORE,
            "GitHub link parsing + egress policy (no live network call)"),
     Script("verify_modelserver", CORE,