Co-Study4Grid / docs /architecture /shared-network-concurrency.md
github-actions[bot]
Deploy 7688ef1
13d4e44
|
Raw
History Blame Contribute Delete
7.36 kB

Concurrency ownership for the shared pypowsybl Network (D3, 2026-07)

Deep revision D3 from 2026-07-full-repo-review.md. Closes cross-cutting theme T3 ("concurrency debt meeting a new deployment reality").

The problem

The backend's central bet — module-level singletons + a single shared pypowsybl.network.Network that every code path variant-switches — was designed for a single-user, single-flight desktop deployment, and that assumption was explicitly documented (docs/performance/history/grid2op-shared-network.md). Three developments invalidated it:

  1. FastAPI runs the def (sync) endpoints on a threadpool, so two HTTP requests genuinely execute in parallel.
  2. The frontend fires Promise.all batches and detached-tab refreshes — several diagram requests land at once.
  3. The 0.8.0 HuggingFace Space adds concurrent visitors sharing one process (module-level singletons).

The shared Network is variant-switched by ~13 entry points. Every switch is individually paired with a finally-restore, but two concurrent switches on the same handle interleave — one request reads flows from the variant another request just switched away from. One path (diagram_mixin._get_contingency_flows) didn't even have the finally-restore, so an exception there left the shared handle stuck on a contingency variant, silently corrupting every later read.

The fix — three primitives

All in expert_backend/services/service_lock.py

  • the wiring on RecommenderService.

1. A re-entrant service network lock

self._network_lock (a threading.RLock) serializes every entry point that variant-switches the shared Network:

  • Sync entry points wear @with_network_lock — they hold the lock for the whole body.
  • Streaming entry points (run_analysis) wear @with_network_lock_stream — the lock is held per resumption (each phase between two yields is internally variant-consistent, so releasing at yield points is safe and keeps a long phase from starving diagram requests any longer than that one phase).

Decorated entry points (13):

Mixin Methods
DiagramMixin get_network_diagram, get_contingency_diagram, get_action_variant_diagram, get_contingency_diagram_patch, get_action_variant_diagram_patch, get_n_sld, get_contingency_sld, get_action_variant_sld, get_topology_preview_sld
AnalysisMixin run_analysis_step1, run_analysis (stream)
SimulationMixin simulate_manual_action, compute_superposition

compute_superpositionsimulate_manual_action is a nested locked call on the same thread — the RLock is re-entrant, so it doesn't deadlock.

Thread-affinity subtlety. Starlette iterates a sync streaming generator via iterate_in_threadpool, which may run each next() on a different worker thread. An RLock must be released by the thread that acquired it, so a naive with lock: yield from ... would break. _LockPerStepIterator acquires and releases inside a single __next__ call — which always runs on one thread — instead.

/api/config holds the lock across the whole reset() → load_network() → update_config() sequence (via the network_lock() context manager) so no diagram request can interleave between the reset and the reload.

2. A study-mutation busy gate → HTTP 409

Study-level operations — /api/config, run-analysis-step1, run-analysis-step2, the legacy run-analysis stream — each take seconds and mutate the shared singleton state wholesale. Queueing a second one behind the first is worse than refusing it, so try_begin_study_mutation() is a non-blocking claim that maps a conflict to HTTP 409 ("another study operation is already in progress"). It's a plain threading.Lock (not an RLock) because a streaming mutation acquires it on the request thread and releases it — in the generator's finally — from whatever threadpool thread finishes the stream.

Read-only diagram/SLD requests are NOT gated: they only need the network lock (serialize), not the busy gate (refuse).

3. Bounded variant + observation lifecycle

Cached contingency variants previously grew without bound within a session (one per contingency ever viewed), each costing pypowsybl-side memory proportional to the grid. _touch_contingency_variant keeps an LRU of at most MAX_CONTINGENCY_VARIANTS (8) contingency variants on the shared Network beyond the N baseline; eviction calls remove_variant and drops the matching _lf_status_by_variant entry. Re-viewing an evicted contingency transparently re-clones and re-runs its AC load flow. The LRU never evicts the N baseline, the variant being returned to the caller, or the one the Network is currently positioned on.

Lock-ordering vs the NAD-prefetch drain

The old reset() joined the in-flight NAD-prefetch thread (join(timeout=60)) to stop it leaking its result into the next study. With the network lock in place that join becomes a deadlock risk: the prefetch worker now takes the same _network_lock around its whole body, so a caller that holds the lock (e.g. /api/config) and then tries to join the worker would wait 60 s for a worker that is itself blocked on the very lock the caller holds.

The join is replaced by a monotonic _prefetch_generation counter: reset() and every new prefetch bump it, and a worker whose captured generation is stale (checked under the lock, before and after its compute) discards its result instead of poisoning the next study's cache. Mutual exclusion is already guaranteed by lock ownership; the counter only handles staleness. _drain_pending_base_nad_prefetch() is therefore a no-op when the service lock is present and falls back to the historical join only for bare-mixin test hosts that never ran RecommenderService.__init__ (no lock).

What is NOT changed

  • Endpoints / frontend: behaviour is identical for a single user; the 409 is only reachable under genuine concurrency.
  • run_analysis_step2 runs on the grid2op env's own network instance, not the shared _base_network, so it takes the study gate (it IS a study mutation) but not the network lock.
  • Bare-mixin tests: the decorators and the drain degrade to no-ops when _network_lock is absent, so isolated mixin tests keep running single-threaded unchanged.

Tests

  • test_service_concurrency.py — lock re-entrancy, cross-thread gate release, decorator serialization, the streaming decorator's per-next() lock release (a long stream must not hold the lock across yields) + no-op fallback, the variant LRU (eviction, reuse reorder, never-evict-working, reset clears it), and the NAD-prefetch generation-staleness discard — a worker whose generation was superseded mid-compute (as reset() does) drops its result instead of poisoning the next study's cache (the behaviour that replaced the deadlock-prone join()).
  • test_api_endpoints.py::TestStudyMutationBusyGate — the 409 contract on config / step-1 / step-2 and gate release on success, error, and after a stream drains.