[26.1] Fix celery worker cold-start thundering herd (build app once, scope tool-data walk to load pass)

[mvdbeek]

On a cold start with celery's thread pool (--pool threads --concurrency=N), every task's before_start hook calls get_galaxy_app() → build_app(). A py-spy dump of a production worker showed all 16 pool threads stuck concurrently in GalaxyManagerApplication.__init__, each building a full Galaxy app — a thundering herd that multiplies cold-start time and memory. This PR removes two distinct causes.

1. Build the Galaxy app once under an explicit lock

build_app() was memoized with @lru_cache(maxsize=1), but that does not collapse the herd: CPython's lru_cache runs the wrapped function outside its internal lock, so N threads all miss the empty cache simultaneously and each builds a full app before the first result is stored. Replaced with an explicit double-checked lock so exactly one thread builds the app and the rest wait and reuse it. (Safe in CPython: the module-global assignment is atomic under the GIL, and nothing calls build_app.cache_clear().)

2. Scope the tool-data directory walk to a load pass

ToolDataPathFiles re-walked the entire tool_data_path whenever its cache was older than 1 second. A full os.walk of a production tool-data tree can take longer than that, so within a single config load pass nearly every exists() check triggered a fresh full walk — a walk storm on its own, and dramatically worse under the app-build herd above.

exists() is only ever called while loading/reloading tables, and all tables in a pass share one ToolDataPathFiles instance, so the listing only needs to live for one pass. Introduced a re-entrant cached() context manager (walk once on enter, share the snapshot, drop it on exit) and wrapped load_from_config_file() and reload_tables() in it. Outside a pass nothing is cached and exists() resolves each path directly via os.path.exists(), so the listing can never outlive the on-disk state it was taken from — avoiding both an indefinite staleness window (a deleted .loc reported as still present) and any reliance on the tool-data directory watcher to keep the cache honest.

How to test the changes?

(Select all options that apply)

• This is a refactoring of components with existing test coverage.
  • test/unit/tool_util/data/ and test/unit/tool_shed/test_data_table_manager.py pass (32 tests); they exercise table loading/reloading through the new cached() scoping.
  • test/unit/app/test_celery.py covers build_app.
• Instructions for manual testing are as follows:
  1. Start celery with --pool threads --concurrency=16 against a config with a large tool_data_path; confirm (e.g. via py-spy) that only one thread runs GalaxyManagerApplication.__init__ and the tool-data tree is walked once per load pass rather than repeatedly.

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[mvdbeek]

CI status

All failing checks on this PR are from a July-1 CI-environment regression (conda won't install: ModuleNotFoundError: No module named 'conda' / is_conda_installed() == False), not from this branch. The same suites pass on dev's last runs and fail identically on unrelated PRs (e.g. #wf_tool_state), where the exact same tests break — including test_data_manager_installation_table_reload, test_resolvers, and the mass test_toolbox_pytest tool-execution errors.

Breakdown:

• Mulled Unit Tests / Test Galaxy packages for Pulsar — conda dependency resolution can't run.
• Integration (0–3) — conda No module named 'conda' → 500s on dependency APIs; data-manager/resolver jobs go to error state.
• Main tool tests — jobs error with exit_code: None (empty output) for even trivial tools like addValue; also fails on unrelated PRs.
• Test Galaxy packages (3.10 / 3.14) — already failing on dev before this branch existed.

This PR only touches two files, and both are clear:

• celery/__init__.py (build the app once under a lock) — celery tasks log succeeded throughout the failing runs; worker app-build works.
• tool_util/data/__init__.py (per-load-pass listing cache) — exists() always falls back to live os.path.exists() when uncached, so no false negatives; the one integration test in its domain (test_data_manager_installation_table_reload) fails identically on a PR that never touched this code.

Everything this PR actually affects is green (API tests, unit suites, startup, DB indexes, schema validation). Reruns don't help while the environment is broken

[domgz]

I think it makes sense that at least one of @domgz, @gsaudade99 or @mira-miracoli take a look at this.

I am especially concerned about the fact that the context manager is used within exists(), and I am not sure how many times the startup process is calling exists().

But that should be easy to figure out because we can patch the code live to count the amount of calls and log them; we should ensure that exists() is called sufficiently few times.

In the meantime the cheap solution seems to be saving our face.

[mvdbeek]

Thanks @domgz — a review from @domgz / @gsaudade99 / @mira-miracoli would be very welcome.

One clarification first, because I think there's a small misreading that changes the risk assessment: the context manager is not used within exists(). exists() only does an O(1) set lookup against an already-built listing, falling back to a single os.path.exists() on a miss. cached() is entered in exactly two places — wrapping the whole load pass in load_from_config_file() and reload_tables() — never per exists() call. So os.walk runs once per load pass and every exists() inside that pass reuses the one snapshot.

I ran the actual startup path to get real numbers (the _configure_tool_data_tables sequence from app/__init__.py:467-484, my dev config, ~65 tables loaded):

Startup tool-data initialization:
  WALK #1 via=load_from_config_file   (tool_data_table_conf)
  WALK #2 via=load_from_config_file   (shed_tool_data_table_conf)
  tables_loaded=65
  tool-data WALKS   = 2
  exists() LOOKUPS  = 427

reload_tables():
  tool-data WALKS   = 1   (for all 65 tables)

So 427 exists() lookups → 2 walks at startup (one per config file), and 1 walk per reload, independent of table/lookup count. That's the point: the walk count is decoupled from exists() call count and spacing — which is exactly the pathology the usegalaxy-eu patch works around (under the 1s TTL a walk fires whenever two lookups are >1s apart, and on NFS during boot they routinely are → "hours and hours of walks"). Bumping the TTL to 10 minutes narrows that window but keeps the coupling and adds a 10-minute staleness window; this change removes both.

I also added regression tests asserting this (test/unit/tool_util/data/test_tool_data.py): one walk per config-load pass, one per reload pass, and zero walks for exists() outside a pass. No objection to the eu 10-minute patch as a stopgap in the meantime.

[nsoranzo]

Test Galaxy packages for Pulsar failure is likely legit.

[jmchilton]

On a re-read - these unit tests actually seem better than I worried. There almost so good I wonder if they worth injecting a constructor argument with an actual update/exists implementation that we can use to write tests without mocking.

[mvdbeek]

Thanks @jmchilton — did exactly that in 3ae3d95. ToolDataPathFiles now takes an injectable filesystem (a small ToolDataFilesystem protocol wrapping the only two operations it performs — walk and exists), defaulting to a real _OsFilesystem so every existing caller is unchanged; ToolDataTableManager threads an optional filesystem= through to it.

The walk-count regression tests now drive that seam with a CountingFilesystem (an actual implementation that counts walks/exists) instead of monkeypatching ToolDataPathFiles. Same assertions — one walk per config-load pass, one per reload pass, zero walks for exists() outside a pass — no mocking.

[domgz]

Sorry @mvdbeek I only had time to glance at it last time. I can see now that this is carefully designed and used so that walks are minimized.

A big thank you, this was quite problematic.