From 3a4fcd908f040db7988d9bf03a5ee461593ef4cf Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 24 Jul 2026 09:15:04 +0200 Subject: [PATCH] Serialize cached tool-search index builds to fix concurrent-rebuild crash reindex_tool_search reaches CachedToolboxSearch.build_index concurrently from app boot, the rebuild_toolbox_search_index control task, and remove_tool_by_id, all writing the same on-disk Whoosh directories. Two builds racing there let one thread's create_in/rmtree pull segment files out from under another, surfacing as whoosh.index.LockError or a fatal FileNotFoundError on the segment TOC that killed TestCachedDataManagerIntegration setup in CI (Integration shard 2). Guard build_index with an RLock on the search singleton so only one rebuild touches the index dirs at a time. Regression test drives eight threads through build_index concurrently; it crashes on the unlocked code and passes with the lock. --- lib/galaxy/tools/search/__init__.py | 24 ++++-- .../source_store/test_search_concurrency.py | 86 +++++++++++++++++++ 2 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 test/unit/app/tools/source_store/test_search_concurrency.py diff --git a/lib/galaxy/tools/search/__init__.py b/lib/galaxy/tools/search/__init__.py index ce73300a9bc..4f4f0bee3b7 100644 --- a/lib/galaxy/tools/search/__init__.py +++ b/lib/galaxy/tools/search/__init__.py @@ -28,6 +28,7 @@ Filters - various filters are available for processing content as the index is import logging import os import shutil +import threading from typing import ( Protocol, runtime_checkable, @@ -136,17 +137,28 @@ class CachedToolboxSearch(ToolBoxSearch): self.cached_panel_searches: dict[str, ToolWhooshIndex] = {} self._panel_view_ids: set[str] = set() self.index_count = -1 + # ``reindex_tool_search`` reaches ``build_index`` concurrently from + # boot, the ``rebuild_toolbox_search_index`` control task, and + # ``remove_tool_by_id`` — all writing the same on-disk Whoosh dirs. + # Two builds racing there corrupt each other: one thread's + # ``ToolWhooshIndex._open`` can ``create_in``/``rmtree`` a dir another + # thread is mid-write on, surfacing as ``whoosh.index.LockError`` or a + # fatal ``FileNotFoundError`` on the segment TOC (observed killing + # TestCachedDataManagerIntegration setup in CI). Serialize the whole + # build so only one rebuild touches the index dirs at a time. + self._build_lock = threading.RLock() if toolbox is not None: self._sync_panel_searches(toolbox) def build_index(self, tool_cache: "ToolCache", toolbox: "ToolBox", index_help: bool = True) -> None: cached_toolbox = self._require_search_toolbox(toolbox) - self._sync_panel_searches(cached_toolbox) - tool_index = cached_toolbox.tool_index - if tool_index is not None: - for panel_view_id, searcher in self.cached_panel_searches.items(): - searcher.build(tool_index, cached_toolbox.panel_view_tool_ids(panel_view_id)) - self.index_count += 1 + with self._build_lock: + self._sync_panel_searches(cached_toolbox) + tool_index = cached_toolbox.tool_index + if tool_index is not None: + for panel_view_id, searcher in self.cached_panel_searches.items(): + searcher.build(tool_index, cached_toolbox.panel_view_tool_ids(panel_view_id)) + self.index_count += 1 def search(self, q: str, panel_view: str, config: GalaxyAppConfiguration) -> list[str]: if panel_view not in self._panel_view_ids: diff --git a/test/unit/app/tools/source_store/test_search_concurrency.py b/test/unit/app/tools/source_store/test_search_concurrency.py new file mode 100644 index 00000000000..3f3a66deca7 --- /dev/null +++ b/test/unit/app/tools/source_store/test_search_concurrency.py @@ -0,0 +1,86 @@ +"""Concurrent ``CachedToolboxSearch.build_index`` calls must not corrupt the index. + +``reindex_tool_search`` reaches ``build_index`` from several threads at once +(boot, the ``rebuild_toolbox_search_index`` control task, and +``remove_tool_by_id``). Without serialization two builds race on the same +on-disk Whoosh directories and one thread's ``create_in``/``rmtree`` pulls the +segment files out from under the other, surfacing as ``whoosh.index.LockError`` +or a fatal ``FileNotFoundError`` on the segment TOC. +""" + +import threading +from types import SimpleNamespace +from typing import cast + +from galaxy.config import GalaxyAppConfiguration +from galaxy.tools.search import CachedToolboxSearch +from galaxy.tools.source_store.index import ( + ToolIndex, + ToolIndexEntry, +) +from galaxy.tools.source_store.search import ToolSearchTuning + +_TUNING = ToolSearchTuning( + id_boost=20.0, + name_boost=10.0, + name_exact_multiplier=2.0, + stub_boost=5.0, + section_boost=4.0, + description_boost=3.0, + label_boost=3.0, + ngram_minsize=3, + ngram_maxsize=4, + enable_ngram_search=True, + ngram_factor=0.5, +) + + +class _FakeCachedToolbox: + """Minimal ``SupportsCachedSearch`` surface over a static index.""" + + def __init__(self, index: ToolIndex, panel_view_ids: list[str]) -> None: + self._index = index + self._panel_view_ids = panel_view_ids + + @property + def tool_index(self) -> ToolIndex: + return self._index + + def panel_views(self): + return [SimpleNamespace(id=view_id) for view_id in self._panel_view_ids] + + def panel_view_tool_ids(self, panel_view_id: str) -> set[str]: + return set(self._index.entries) + + +def test_concurrent_build_index_does_not_corrupt(tmp_path, search_config): + index = ToolIndex() + for name in ("mapper", "caller", "trimmer", "aligner", "sorter"): + index.add_entry(ToolIndexEntry(id=name, version="1.0", name=name.title())) + # Several panel views multiply the on-disk dirs each build touches, widening + # the window two racing builds can collide in. + toolbox = _FakeCachedToolbox(index, ["default", "my_panel", "ontology:edam_operations"]) + config = search_config(_TUNING, index_dir=str(tmp_path / "tool_search_index")) + search = CachedToolboxSearch(cast(GalaxyAppConfiguration, config), toolbox=toolbox) + + errors: list[Exception] = [] + start = threading.Barrier(8) + + def build_repeatedly() -> None: + start.wait() + try: + for _ in range(6): + search.build_index(tool_cache=None, toolbox=toolbox) # type: ignore[arg-type] + except Exception as exc: + errors.append(exc) + + threads = [threading.Thread(target=build_repeatedly) for _ in range(8)] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=30) + + assert all(not thread.is_alive() for thread in threads) + assert not errors, f"Concurrent build_index raised: {errors!r}" + # The index is intact and queryable after the concurrent hammering. + assert search.search("mapper", "default", config) == ["mapper"]