Skip to content

Commit bcde11d

Browse files
docs: search-index lock-acquisition order and threading model (#138)
* docs: search-index lock-acquisition order and threading model * docs: clarify search-index publish and index_locked semantics * docs: clarify NO_SEARCH_INDEX leaves index files on disk * Address timon's feedback * Address Will's feedback
1 parent 0c43bd7 commit bcde11d

2 files changed

Lines changed: 63 additions & 1 deletion

File tree

README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,18 @@ python app.py --base-dir /path/to/claude/projects
7171
> The server **refuses to start** if `--debug` is combined with a non-loopback `--host` (e.g. `0.0.0.0`).
7272
> That check runs only when you start the app with **`python app.py`** (not via `flask run` or other WSGI entrypoints).
7373
74+
#### Deployment: WSGI workers
75+
76+
The FTS search index (`utils/search_index.py`) assumes **a single Python process** serves the app:
77+
78+
- One daemon thread rebuilds the index and publishes it by atomically swapping `search_index.active` (see [Search index — locks and threading](docs/architecture.md#search-index-fts5--locks-and-threading)).
79+
- Module-level locks and the in-memory usability cache are **per process**, not shared across workers.
80+
- A cross-process advisory lock (`search_index.background.lock`) ensures at most **one** background refresher per index directory on a host, but **does not** coordinate search reads or caches across workers.
81+
82+
**Do not run multiple Gunicorn/uWSGI workers** (`gunicorn -w 2`, etc.) against one operator-facing deployment. Requests will hit different processes with divergent cache state; only one worker runs background refresh; index freshness and `index_locked` / live-scan fallback become nondeterministic. Use **`--workers 1`** (or run `python app.py`) for supported behavior. For horizontal scale, run separate single-worker instances with disjoint `CLAUDE_CODE_CHAT_BROWSER_SEARCH_INDEX_DIR` and workspace paths — not multiple workers behind one load balancer on the same paths.
83+
84+
Set `CLAUDE_CODE_CHAT_BROWSER_NO_SEARCH_INDEX=1` to skip building and querying the FTS index (live JSONL scan only; slower). Existing files under the search index directory are left on disk.
85+
7486
### CLI Export
7587

7688
```bash

docs/architecture.md

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
3. User opens a project → `GET /api/projects/<name>/sessions` → full `parse_session()` per file, exclusion filter, summary rows.
6767
4. User opens a session → `GET /api/sessions/<project>/<id>` → full session JSON for the message panel.
6868
5. Optional: `GET /api/sessions/.../stats` for sidebar metrics without loading all messages.
69-
6. Search: `GET /api/search?q=...` scans all projects (brute force).
69+
6. Search: `GET /api/search?q=...` queries the local FTS index when usable, with live JSONL scan fallback (see [Search index](#search-index-fts5--locks-and-threading)).
7070
7. Export: `POST /api/export` or `GET /api/export/session/...` → Markdown/zip via exporters; state file updated on successful bulk export.
7171

7272
## Dispatch table
@@ -143,6 +143,56 @@ No bundler step — modern browsers load modules directly. Frontend unit tests u
143143
- `integration-tests` — API integration subset + coverage artifact
144144
- `js-tests``npm ci` + vitest
145145

146+
## Search index (FTS5) — locks and threading
147+
148+
`utils/search_index.py` maintains a local SQLite FTS5 index under `~/.claude-code-chat-browser/` (override with `CLAUDE_CODE_CHAT_BROWSER_SEARCH_INDEX_DIR`). Session JSONL on disk remains the source of truth; `GET /api/search` can fall back to a live scan when the index is missing, stale, or when a query returns `index_locked` after `sqlite3.OperationalError`.
149+
150+
### Threading model
151+
152+
| Role | Who | What |
153+
|------|-----|------|
154+
| **Readers** | Flask request threads (`GET /api/search` via `query_index_hits`; `index_is_usable` for whether to use the index) | Open the **active** index DB read-only via `_resolve_active_index_db_path()`; `query_index_hits` does not take any module `threading.Lock`. No request handler calls `ensure_search_index` or `build_search_index`. |
155+
| **Writer** | One daemon thread (`search-index-refresh`, started from `start_search_index_background` in `app.py`) | Periodically calls `ensure_search_index``build_search_index` when the projects fingerprint changes. |
156+
| **Publish** | Writer, at end of a successful rebuild | `_publish_active_index` writes the new DB basename to `search_index.active.tmp`, then tries `Path.replace` onto `search_index.active`. On `OSError`, it falls back to `pointer.write_text` (not rename-atomic on every platform). Readers resolve the active **SQLite file** by name from the pointer; sidecar DB files are fully committed before publish. After a successful `replace`, readers see the previous or new complete DB, not a half-written SQLite file. The direct-write fallback can briefly expose a partially written pointer file, so treat `replace` as the normal path. |
157+
158+
Rebuild work happens on a **new** SQLite file while the prior active DB file stays on disk until publish; that isolation does **not** by itself lock the old database or force readers onto the in-progress file. Separately, `query_index_hits` maps `sqlite3.OperationalError` from the FTS query (for example `database is locked` on the DB it opened) to `index_locked=True` so `/api/search` can degrade to live scan — that flag is a **query-time classification** for fallback, not proof that a rebuild is in progress. Concurrency tests should assert both behaviors without conflating them.
159+
160+
### Lock inventory
161+
162+
Four module-level synchronization primitives guard index lifecycle and caches (`utils/search_index.py`):
163+
164+
| Primitive | Type | Guards |
165+
|-----------|------|--------|
166+
| `_index_lock` | `threading.Lock` | `_background_started` and startup/teardown of the background worker (`start_search_index_background`, `reset_background_for_tests`). |
167+
| `_index_build_lock` | `threading.Lock` | Entire `build_search_index` body — at most one rebuild at a time **in this process**. |
168+
| `_background_lock_fd` | OS advisory lock on `search_index.background.lock` | Cross-process **single background owner** for a shared index directory (`_try_acquire_cross_process_background_lock`). |
169+
| `_usability_cache_lock` | `threading.Lock` | In-memory `_usability_cache` used by `index_is_usable` and cleared after rebuild (`_clear_usability_cache`). |
170+
171+
**Usage map (current code):**
172+
173+
- `_index_build_lock` — wraps `build_search_index` (serializes rebuild and fingerprint short-circuit reads).
174+
- `_index_lock` — held briefly while deciding whether to start the daemon and while resetting test state; may call `_try_acquire_cross_process_background_lock` while held.
175+
- `_background_lock_fd` — non-blocking exclusive flock/msvcrt lock; failure means another process already owns background refresh for this cache dir.
176+
- `_usability_cache_lock``index_is_usable` and `_clear_usability_cache`.
177+
- `_publish_active_index` — pointer update (`replace` with `write_text` fallback) + `_prune_stale_index_files` (no module lock; build serialization via `_index_build_lock`).
178+
179+
### Lock-acquisition contract (must stay so)
180+
181+
The four primitives are **not nested arbitrarily**. New code must preserve this contract; the planned Week 30 concurrency suite (`tests/test_search_index_concurrency.py`, PR after this doc) will assert it and fail if lock order regresses.
182+
183+
1. **`_index_lock`** may be taken alone, or together with acquiring the advisory background lock during `start_search_index_background`. Do **not** call `build_search_index` while holding `_index_lock`.
184+
2. **`_index_build_lock`** is taken only inside `build_search_index`. While held, code may open the active DB read-only and write a **new** sidecar SQLite file. The only permitted nested module lock is **`_usability_cache_lock`** via `_clear_usability_cache` at the end of a successful rebuild (`_index_build_lock``_usability_cache_lock`).
185+
3. **`_usability_cache_lock`** must never be held while waiting on `_index_build_lock` or `_index_lock`. Typical pattern: short cache read/update in `index_is_usable` without holding the build lock.
186+
4. **`_background_lock_fd`** is independent of the three `threading.Lock` instances except at startup: acquire the advisory lock only from `_try_acquire_cross_process_background_lock`, which is called under `_index_lock` once per process.
187+
188+
**Forbidden:** holding `_usability_cache_lock` and then acquiring `_index_build_lock` or `_index_lock`; holding `_index_lock` across `build_search_index`; taking `_index_build_lock` from multiple threads without going through the existing `build_search_index` entry point.
189+
190+
### Deployment assumptions
191+
192+
The design targets **one WSGI worker process** (for example `python app.py` or `gunicorn --workers 1`). That gives one in-process writer thread, one advisory-lock owner, and consistent in-memory usability cache per operator session.
193+
194+
See [Deployment: WSGI workers](../README.md#deployment-wsgi-workers) in the README for multi-worker warnings.
195+
146196
## What this codebase is not
147197

148198
- **Not multi-user** — no authn/authz; single local operator.

0 commit comments

Comments
 (0)