From 4f2d9dd47b0bc6250747ce9702ff959467d14f1b Mon Sep 17 00:00:00 2001 From: mikessh Date: Wed, 28 Oct 2026 02:36:16 +0300 Subject: [PATCH 1/3] Serialize control builds without optional dependencies Co-Authored-By: Codex --- .github/workflows/ci.yml | 7 ++++ docs/evalue.rst | 8 +++-- python/seqtree/control.py | 63 ++++++++++++++++++++++++++---------- tests/python/test_control.py | 55 +++++++++++++++++++++++++++++++ 4 files changed, 113 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 21e8d3c..722d4d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,9 @@ jobs: matrix: os: [ubuntu-latest, macos-latest] python: ["3.10", "3.12"] + include: + - os: windows-latest + python: "3.12" steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 @@ -46,7 +49,11 @@ jobs: continue-on-error: true run: uv pip install --system olga - name: Test + if: runner.os != 'Windows' run: pytest tests/python -q + - name: Windows control locking + if: runner.os == 'Windows' + run: pytest tests/python/test_control.py -q benchmarks: name: Benchmark tables + plots (oracle + perf) diff --git a/docs/evalue.rst b/docs/evalue.rst index 9cb6fef..9a65db5 100644 --- a/docs/evalue.rst +++ b/docs/evalue.rst @@ -68,9 +68,11 @@ precision bound below. That matters on *first* use in any multi-process fan-out — pytest-xdist, a Snakemake or Nextflow rule, a ``multiprocessing`` pool — all sharing one ``~/.cache``. Once the cache is warm it is - read-only and was never at risk. If ``filelock`` is installed (it arrives with - ``huggingface_hub``) the build is additionally serialized, so one worker builds and the rest - wait and load rather than all N rebuilding the same index; correctness does not depend on it. + read-only. Cold builds are serialized with an operating-system lock, including in a + base installation without ``filelock``. One worker builds and the rest wait and load. + Waiting is bounded to 600 seconds; an active owner causes a clear timeout. The kernel + releases ownership after an exception or process exit. Persistent ``.lock`` guard files + are intentional and must not be deleted while other processes may be waiting. A cache that is corrupt, truncated, or written by an older seqtree is silently rebuilt. diff --git a/python/seqtree/control.py b/python/seqtree/control.py index b53aff0..640de03 100644 --- a/python/seqtree/control.py +++ b/python/seqtree/control.py @@ -122,24 +122,53 @@ def _prune_superseded(cache_dir, name, size, keep): pass -def _build_lock(cache): - """An inter-process lock around build-and-save, if ``filelock`` happens to be installed. - - This is an *optimisation*, not the fix. ``Index.save`` writes to a temporary and renames it - into place, so a reader can never observe a half-written index whether or not this lock is - taken. What the lock buys is work: without it, a cold-cache fan-out of N workers has every - worker build the same 250k-clonotype index (seconds of CPU, 45 MB written) and N-1 of those - are thrown away. With it, one builds and the rest wait and load. - - seqtree has no runtime dependencies, so ``filelock`` is used only if something else already - put it there -- ``huggingface_hub`` pulls it in, and that is installed whenever a control is - downloaded rather than read from the bundled asset. +@contextlib.contextmanager +def _build_lock(cache, timeout=600): + """Serialize cold builds without optional dependencies; process death releases the lock. + + Keep the guard file: deleting it could let waiters lock different inodes. """ - try: - from filelock import FileLock - except ImportError: - return contextlib.nullcontext() - return FileLock(cache + ".lock", timeout=600) + import errno + import time + + with open(cache + ".lock", "a+b") as handle: + if os.name == "nt": + import msvcrt + if handle.seek(0, 2) == 0: + handle.write(b"\0") + handle.flush() + + def acquire(): + handle.seek(0) + msvcrt.locking(handle.fileno(), msvcrt.LK_NBLCK, 1) + + def release(): + handle.seek(0) + msvcrt.locking(handle.fileno(), msvcrt.LK_UNLCK, 1) + else: + import fcntl + + def acquire(): + fcntl.flock(handle.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + + def release(): + fcntl.flock(handle.fileno(), fcntl.LOCK_UN) + + deadline = time.monotonic() + timeout + while True: + try: + acquire() + break + except OSError as exc: + if exc.errno not in (errno.EACCES, errno.EAGAIN, errno.EDEADLK): + raise + if time.monotonic() >= deadline: + raise TimeoutError(f"Another control build owns {cache}.lock after {timeout:g}s") from exc + time.sleep(0.05) + try: + yield + finally: + release() def _load_cached(cache): diff --git a/tests/python/test_control.py b/tests/python/test_control.py index a4ab6d4..8aa5083 100644 --- a/tests/python/test_control.py +++ b/tests/python/test_control.py @@ -178,3 +178,58 @@ def test_a_superseded_cache_is_not_served_and_is_cleaned_up(tmp_path): assert not legacy.exists(), "the superseded cache was left behind" assert ([p.name for p in tmp_path.iterdir() if p.suffix == ".sqtree"] == [control._cache_key("human_trb_aa", n, "aa", 0)]) + + +def test_control_build_lock_without_optional_filelock(tmp_path, monkeypatch): + import sys + import threading + import time + from seqtree.control import _build_lock + + monkeypatch.setitem(sys.modules, "filelock", None) + ready = threading.Barrier(2) + state = {"active": 0, "peak": 0} + errors = [] + + def worker(): + try: + ready.wait(timeout=5) + with _build_lock(str(tmp_path / "control.idx")): + state["active"] += 1 + state["peak"] = max(state["peak"], state["active"]) + time.sleep(0.1) + state["active"] -= 1 + except BaseException as exc: + errors.append(exc) + + workers = [threading.Thread(target=worker) for _ in range(2)] + for worker_thread in workers: + worker_thread.start() + for worker_thread in workers: + worker_thread.join(timeout=5) + assert not errors + assert not any(worker_thread.is_alive() for worker_thread in workers) + assert state["peak"] == 1 + assert state["active"] == 0 + + +def test_control_lock_releases_after_failure_and_process_exit(tmp_path): + import subprocess + import sys + import pytest + from seqtree.control import _build_lock + + cache = str(tmp_path / "control.idx") + with pytest.raises(RuntimeError, match="failed build"): + with _build_lock(cache): + raise RuntimeError("failed build") + with _build_lock(cache): + with pytest.raises(TimeoutError, match="Another control build"): + with _build_lock(cache, timeout=0.02): + pass + subprocess.run([sys.executable, "-c", + "import os,sys; from seqtree.control import _build_lock; " + "lock=_build_lock(sys.argv[1]); lock.__enter__(); os._exit(0)", cache], + check=True, timeout=10) + with _build_lock(cache, timeout=1): + pass From 9b06a2a4e531c156b1f099547406953d11c58c6d Mon Sep 17 00:00:00 2001 From: mikessh Date: Wed, 28 Oct 2026 02:40:17 +0300 Subject: [PATCH 2/3] Assert dependency-free locking behavior in cache regression Co-Authored-By: Codex --- tests/python/test_cache_concurrency.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/tests/python/test_cache_concurrency.py b/tests/python/test_cache_concurrency.py index 42b3d2e..87a9972 100644 --- a/tests/python/test_cache_concurrency.py +++ b/tests/python/test_cache_concurrency.py @@ -169,19 +169,11 @@ def test_a_corrupt_cache_is_rebuilt_rather_than_raised(tmp_path): assert len(seqtree.Index.load(str(cache))) == n # and the cache was rewritten in place -def test_the_lock_is_optional_and_correctness_does_not_depend_on_it(monkeypatch): - """A bare ``pip install seqtree`` has no filelock -- it arrives only with huggingface_hub. - - The lock saves redundant work; the atomic save is what makes concurrent use *correct*. If this - ever regresses into a hard import, a default install starts raising ImportError on first use. - """ +def test_the_lock_does_not_require_optional_filelock(tmp_path, monkeypatch): + """A base install serializes builds without importing an optional dependency.""" import builtins - from seqtree import control - assert control._build_lock("/tmp/x").__class__.__name__ in {"FileLock", "UnixFileLock", - "WindowsFileLock", "nullcontext"} - real_import = builtins.__import__ def no_filelock(name, *args, **kwargs): @@ -190,7 +182,10 @@ def no_filelock(name, *args, **kwargs): return real_import(name, *args, **kwargs) monkeypatch.setattr(builtins, "__import__", no_filelock) - lock = control._build_lock("/tmp/x") - assert lock.__class__.__name__ == "nullcontext" - with lock: # must be a usable context manager, not a stub that explodes + cache = str(tmp_path / "control.idx") + with control._build_lock(cache): + with pytest.raises(TimeoutError): + with control._build_lock(cache, timeout=0): + pass + with control._build_lock(cache, timeout=0): pass From b86eaf0df538275e3b30bcf2704882f53bd8f025 Mon Sep 17 00:00:00 2001 From: mikessh Date: Wed, 28 Oct 2026 02:50:20 +0300 Subject: [PATCH 3/3] Prepare seqtree 1.3.2 patch release Co-Authored-By: Codex --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f0b6761..8616a7b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "seqtree" -version = "1.3.1" +version = "1.3.2" description = "Fast fuzzy search over biological sequences (C++ core, Python bindings)" readme = "README.md" requires-python = ">=3.10"