mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-01 15:37:32 +08:00
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.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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"]
|
||||
Reference in New Issue
Block a user