Skip to content

fix(DISET): prevent services from getting permanently stuck in throttle mode - #8471

Open
ryuwd wants to merge 6 commits into
DIRACGrid:integrationfrom
ryuwd:roneil-throttle-recovery
Open

fix(DISET): prevent services from getting permanently stuck in throttle mode#8471
ryuwd wants to merge 6 commits into
DIRACGrid:integrationfrom
ryuwd:roneil-throttle-recovery

Conversation

@ryuwd

@ryuwd ryuwd commented Feb 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Production DISET services can enter throttle mode and never recover, requiring manual restarts. This PR stops the queue growing while a service is overloaded, adds diagnostics, and adds an opt-in auto-restart for services that stay stuck.

  • Reject all connections while throttled: previously, once the queue exceeded MaxWaitingPetitions, connections were rejected for a 0.25s window and then one was accepted, so roughly 4 req/s still got through. When threads were stuck (blocked DB queries, deadlocks, etc.) the queue kept growing, and by the time queued requests were processed their clients had long since timed out. Now all connections are rejected while throttled, which bounds the queue. This does not unstick stuck threads; that is what the auto-restart is for.
  • Hysteresis: throttling starts when the queue exceeds MaxWaitingPetitions and only clears once it has drained to half of that. This avoids flapping in and out of throttle mode (and the log spam that comes with it) when rejected clients immediately reconnect.
  • Diagnostic logging: throttle state (start time, last periodic log) lives on the Service instance, so it persists across __acceptIncomingConnection re-entries. Logs WARN on entry with queue/thread stats, WARN every 30s while it persists, and INFO on clear with the total duration.
  • Configurable auto-restart: new MaxThrottleDuration CS option (default: 0 = disabled). When exceeded, the service logs a FATAL message, flushes its log handlers and exits with os._exit(1) so the process supervisor (runsv) restarts it. A normal interpreter exit cannot be used: it joins the ThreadPoolExecutor workers, which hangs forever when they are stuck. In a clone process (CloneProcesses > 1), the clone signals the parent so the whole service restarts rather than silently losing a clone.

Operational Impact

Behavioural change

Previously, a throttled service would still accept ~4 connections/s. Now it rejects all connections until the queue has drained to half of MaxWaitingPetitions. This means:

  • Clients see rejections sooner during genuine overload, rather than queueing behind stuck threads
  • Transient load spikes still recover naturally
  • The queue can no longer grow without bound behind stuck threads

New logging

  • WARN on throttle entry with queue=X/Y, threads=X/Y diagnostics
  • WARN every 30s while throttling persists, with updated stats
  • INFO when throttle clears, with total duration
  • Existing per-rejection WARN is preserved for monitoring

Auto-restart

  • Disabled by default (MaxThrottleDuration = 0)
  • Opt-in per service via CS: Systems/<System>/Services/<Service>/MaxThrottleDuration = 600
  • Read once at service start-up, so changing it requires a service restart
  • Requires a process supervisor (runsv, systemd, etc.) to restart the service
  • Only evaluated when a connection arrives; a stuck service with no clients is left alone

Recommended deployment

  1. Deploy with auto-restart disabled (default) to validate the throttle fix
  2. Monitor logs for throttle duration patterns
  3. Enable MaxThrottleDuration for services known to get stuck (suggested starting value: 600s)

Test plan

  • Unit tests for the throttle state tracking and hysteresis (Test_ServiceThrottle.py)
  • Certification: handler that blocks forever with MaxThrottleDuration set, check the service is restarted by runsv (single process and with CloneProcesses > 1)
  • Hotfix into a service which throttles frequently and validate behaviour

BEGINRELEASENOTES
*Core
FIX: prevent DISET services from getting permanently stuck in throttle mode by rejecting all connections while overloaded instead of accepting one every 0.25s
NEW: add throttle duration tracking with diagnostic logging (queue/thread stats on entry, periodic warnings, clear notification)
NEW: add configurable MaxThrottleDuration CS option to auto-restart services stuck in throttle mode (default: disabled)
ENDRELEASENOTES

@ryuwd
ryuwd force-pushed the roneil-throttle-recovery branch from 06a5f45 to 6c44a2e Compare February 26, 2026 12:40
@aldbr
aldbr requested review from atsareg and chaen and removed request for atsareg February 26, 2026 12:54
@aldbr
aldbr marked this pull request as ready for review March 10, 2026 11:49
@aldbr
aldbr requested review from atsareg and fstagni as code owners March 10, 2026 11:49
@fstagni

fstagni commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

Was this PR hotfixed somewhere, or at least tested in the certification setup?

@ryuwd

ryuwd commented Mar 18, 2026

Copy link
Copy Markdown
Contributor Author

It's not been hotfixed or tested in certification yet

@fstagni

fstagni commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

hi @ryuwd is this PR still relevant?

@ryuwd

ryuwd commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

I don't believe any of the bugs that I aimed to fix here have been addressed, so I think it's still relevant, but it needs the input of e.g. @chaen

ryuwd added 5 commits October 15, 2026 12:18
@ryuwd
fix(DISET): reject all connections during throttling to prevent queue…
5a0bf4c 
… growth

The previous throttle mechanism accepted one connection every 0.25s
even while the service was overloaded. When threads were stuck (e.g.
blocked on DB queries or deadlocked), each accepted connection added
to the already-full queue, making recovery impossible.

Now all incoming connections are rejected while wantsThrottle is True,
with a brief sleep to avoid busy-spinning. This prevents the
self-reinforcing stuck state where the queue grows faster than it
can drain.
@ryuwd
feat(DISET): add throttle duration tracking and diagnostic logging
a069a79 
Track when throttling starts and log state transitions with
queue/thread diagnostics. Periodic warnings are emitted every 30s
while throttling persists, making it easier to diagnose stuck
services from logs without needing to attach a debugger.

Adds Service.throttleDiagnostics() to expose queue size, max queue,
active threads, and max threads for logging purposes.
@ryuwd
feat(DISET): auto-restart service after configurable max throttle dur…
c20a154 
…ation

When a service is stuck in throttle mode (all threads blocked, queue
full), it cannot recover without external intervention. Add a
configurable MaxThrottleDuration CS option (default: 0 = disabled)
that triggers a process exit after the specified number of seconds of
continuous throttling. The process supervisor (e.g. runsv) then
restarts the service cleanly, clearing all stuck state.

When enabled, a FATAL log message is emitted before exit with full
queue/thread diagnostics for post-mortem analysis.
@ryuwd
refactor(DISET): remove throttle sleep to speed up rejection and reco…
ebda9d5 
…very

The 0.25s sleep between throttle rejections was a leftover from the
old "accept 1 every 0.25s" mechanism. With the new reject-all
approach, sel.select(timeout=10) already rate-limits when no
connections arrive. Removing the sleep lets the service reject
incoming connections immediately (faster client failover) and
re-check wantsThrottle sooner (faster recovery when queue drains).
@ryuwd
refactor(DISET): move throttle state tracking into Service
341e1a5 
Throttle state (start time, last log time) was local to
__acceptIncomingConnection, which resets on every re-entry from
serve() (e.g. after a 10s select timeout). This meant
MaxThrottleDuration could never accumulate past 10s if connections
arrived slowly.

Move tracking into Service instance state alongside wantsThrottle,
which now handles entry/exit logging and periodic diagnostics.
Add throttleDuration property for the reactor to check against
MaxThrottleDuration. The ServiceReactor throttle block is simplified
to just rejection + max duration check.
@ryuwd
ryuwd force-pushed the roneil-throttle-recovery branch from 1a435ec to 2f8205f Compare October 15, 2026 10:31
@ryuwd
ryuwd requested a review from andresailer as a code owner October 15, 2026 10:31
@ryuwd
fix(DISET): make throttle auto-restart actually exit and add hysteresis
579e16b 
- Exit with os._exit after flushing logs when MaxThrottleDuration is
  exceeded. A normal exit hangs joining the stuck ThreadPoolExecutor
  workers, so the supervisor never restarted the service.
- In clone processes, signal the parent so the whole service restarts
  instead of silently losing the clone.
- Only clear throttling once the queue drains to half of
  MaxWaitingPetitions, to avoid flapping and log spam.
- Rename the side-effecting wantsThrottle property to updateThrottleState().
- Read MaxThrottleDuration once at service start-up instead of on every
  rejected connection.
- Don't mutate the process list while iterating in stopAllProcess.
- Document MaxThrottleDuration and add unit tests for the throttle state.
@ryuwd
ryuwd force-pushed the roneil-throttle-recovery branch from 2f8205f to 579e16b Compare October 15, 2026 10:32

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chaen chaen Awaiting requested review from chaen
@fstagni fstagni Awaiting requested review from fstagni fstagni is a code owner
@atsareg atsareg Awaiting requested review from atsareg atsareg is a code owner
@andresailer andresailer Awaiting requested review from andresailer andresailer is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ryuwd @fstagni @aldbr