## Goal Replace the per-call cursor lifecycle introduced by #78 with a **thread-local cursor**, lazily created on first use per thread and reused for that thread's lifetime. Same correctness guarantees as #78 (no cursor sharing across threads), but the cursor create/close cost amortises across the thread's many calls instead of per call. This is a perf follow-up to #78. Order: lands **after** the v0.28.0 release from #79 — ship correctness first, perf claw-back as v0.28.1 / v0.29.0. ## Why #78's bench showed: - **Single-threaded** `single_code_map` +25% slower than v0.27.2 baseline (per-call `conn.cursor()` dominates a trivial-SELECT workload). - **Concurrent** ops/sec down 15–30% at 8–32 workers (cursor lifecycle on the hot path). Best-in-class DB libraries (SQLAlchemy, psycopg, asyncpg) don't recreate a cursor per statement. The two common patterns are connection pools (SQLAlchemy) and thread-local cursors (psycopg in worker-thread setups). DuckDB's docs say "separate cursors for concurrent use" — they require isolation, not lifecycle churn. Thread-local satisfies isolation while amortising the cost. ## Design `Registry` gets a `threading.local()` holding the per-thread cursor. The `_read_cursor()` context manager becomes a property-style accessor that returns the calling thread's cursor (creating it on first call): ```python class Registry: def __init__(self, ...) -> None: ... self._thread_cursors = threading.local() def _read_cursor(self) -> duckdb.DuckDBPyConnection: cur = getattr(self._thread_cursors, "cur", None) if cur is None: cur = self._require_open().cursor() cur.execute("SET search_path = 'sdmx'") self._thread_cursors.cur = cur return cur ``` Call sites change from `with self._read_cursor() as cur:` to `cur = self._read_cursor()` — flatter, no context manager. Cursor lives for the thread's lifetime. Same change in `_cur(conn)` helpers (`lazy.py`, `code_query.py`, `hierarchy_query.py`). These get a `_per_thread_cursor(conn)` helper backed by a module-level `WeakKeyDictionary[duckdb.DuckDBPyConnection, threading.local]` so we can key by the registry's connection without leaking when registries get GC'd. `DataStore.query_stream` similarly switches to the thread-local cursor. ## Lifecycle / leak handling `threading.local()` storage is cleaned up automatically when a thread terminates — Python's GC drops the per-thread data, and DuckDB closes the cursor via its finaliser. So the natural lifecycle handles thread death without manual tracking. For the WeakKeyDictionary path (lazy facades): when a `Registry`'s connection is closed/GC'd, the entry vanishes from the dict and the `threading.local()` for that connection's per-thread cursors becomes unreferenced. Cursors close. Edge case to handle: `Registry.close()` should explicitly drop the `threading.local()` and close any live cursors on it — don't wait for GC. The thread-local storage is process-wide, so a registry reopen needs to start fresh. ## FastAPI / uvicorn — can we observe thread lifecycle from outside sdmxlib? **No, and we don't need to.** Quick rundown: - FastAPI `def` route handlers run via `anyio.to_thread.run_sync()` on a bounded threadpool (default 40 threads in anyio). - Threads in that pool are **long-lived workers**, parked between tasks. They are not recycled per-request. - The pool size only changes on explicit `anyio.to_thread.current_default_thread_limiter().total_tokens = N` calls, or on process shutdown. Implication: on a typical FastAPI deployment, a `Registry` ends up with ~40 thread-local cursors per process, stable for the process lifetime. No churn, no leak, no per-request overhead. The exact case thread-local was designed for. Sdmxlib can't subscribe to anyio's thread events, but it doesn't matter — Python's threadlocal storage handles cleanup itself, and `Registry.close()` handles the shutdown side. We can optionally expose a diagnostic `Registry.cursor_pool_info() -> dict[str, int]` returning `{"thread_cursors": N, "current_thread_has_cursor": bool}` for ops dashboards. **Optional**; gate on whether it's wanted. ## Ergonomics — single-developer vs web-app use sdmxlib should serve two use cases well: **Single-developer analysis** (notebook / one-off script — single main thread): - One thread = one cursor, created on first read, reused forever. - No per-call cursor cost. Same behaviour as pre-#78 from the user's point of view. - No API surface change. Win. **Long-running web app** (FastAPI/Flask/etc. — pool of worker threads): - ~N threads = ~N cursors, each created lazily on first read in that thread. - Bounded by pool size. No per-call churn after the first warm-up call per thread. - Concurrent reads remain isolated (each thread has its own cursor). Same correctness as #78. - Cleanup is automatic on thread death; `Registry.close()` cleans the rest. **The two use cases land on the same code path** — no opt-in flag, no special handling. That's the API-ergonomics win over a pool-based design (which would force every caller through a `with reg.connection() as conn:` boundary even in the single-thread notebook case). ## Tests required In `tests/unit/test_concurrent_reads.py`: - [ ] **Existing #76 repro tests must still pass** deterministically (10/10 over repeated invocations). - [ ] **Cursor-reuse invariant**: spawn 1 thread, do 1000 `reg.get(urn)`, assert `cursor_pool_info()` shows exactly 1 cursor created (not 1000). - [ ] **Bounded by thread count**: 32-worker pool doing 1000 gets each, assert cursor count stays at 32 (not 32000). New `tests/unit/test_cursor_lifecycle.py`: - [ ] **No leak across short-lived threads**: spawn 200 ephemeral threads, each doing 1 `reg.get(urn)`, force GC, assert cursor count drops to ≤ active-thread-count + small slack. Validates the leak claim from the MR !39 review thread. - [ ] **Registry.close() drops thread-local storage**: open, read on threads A and B, close registry, assert reading after close raises the lifecycle error (not a stale-cursor crash). - [ ] **Registry reopen starts fresh**: close, reopen, read on the same thread — should work without ghost cursor from the previous incarnation. - [ ] **WeakKeyDictionary GC** (for lazy facades): create+drop many LazyCodelist instances on a thread, assert the per-thread cursor dict doesn't grow unboundedly. In `tests/unit/test_storage_query_bounds.py`: - [ ] **`CountingCursor` still measures right**: the `_instrument` helper from #78 wraps the connection and follows spawned cursors. Verify it still sees every read after the switch to thread-local. May need to update if the cursor-creation path changes shape. ## Bench targets Re-run `just bench` against #77's pinned v0.27.2 baseline. Expectations: | workload | current (#78) | target (this issue) | |---|---|---| | `single_get_dataflow_cold` median | +5–6% | within ±2% of baseline | | `single_code_map` median | +25% | within ±5% of baseline | | `concurrent_get_mixed_8w` ops/sec | −15–30% | within ±5% of baseline | | `concurrent_cache_hit_32w` ops/sec | −30–45% | within ±10% of baseline (target ≥ baseline if possible) | | Errors at all worker counts | 0 | 0 (unchanged) | If targets are met, baseline can be re-pinned via `just bench-update` (separate explicit step, mentioned in the MR description). ## Out of scope (do not pull into this issue) - **Connection pool** (the SQLAlchemy answer). Different design, bigger change, only pays off if someone's load profile pushes past DuckDB's per-connection serialisation. File separately if it surfaces in production. - **Async-native API** (`async def get`). Out of scope — DuckDB is sync; we'd need `asyncio.to_thread` wrapping at every call. - **Concurrent writes**. Still single-threaded; this issue does not change that. - **`docs/concurrency.md` update** describing the thread-local model — should fold into this issue's docs section when it lands (single doc edit). ## References - Bug: #75 - Phases: #76 (repros, merged !37), #77 (baseline, merged !38), #78 (per-call cursor fix, MR !39), #79 (docs + v0.28.0 release). - DuckDB cursor docs: https://duckdb.org/docs/api/python/dbapi#using-cursor - Comparison context: SQLAlchemy uses connection pooling; psycopg2 in worker-thread setups uses thread-local cursors; asyncpg uses a connection pool. None do "fresh cursor per statement." ## Acceptance - All tests above pass. - Bench targets met or documented & accepted. - `docs/concurrency.md` updated to describe the thread-local model and the (former) per-call model as a footnote. - v0.28.1 or v0.29.0 cut after merge.