Skip to content

fix: expire concurrency locks so a missed release cannot deadlock an API - #121

Merged
k3scat merged 1 commit into
devfrom
fix/restore-console-script
Jul 20, 2026
Merged

k3scat merged 1 commit into
devfrom
fix/restore-console-script

Conversation

@k3scat

@k3scat k3scat commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

With key_concurrency: false a key is locked for the duration of a request, and nothing ever expired that lock. Any path that failed to release one took the credential out of rotation for the lifetime of the process; once every key had leaked, the API stopped serving entirely and stayed that way.

The reported symptom was a queue stuck for 30 minutes logging a wait that never changed:

DEBUG | nya.core.queue - Resource Unavailable for novelai endpoint - wait time: 0.50s

That constant is diagnostic. time_to_key_ready treats a locked key as inf, and falls through to a hardcoded return 1.0 when every key is inf, which the queue halves to 0.50s. A rate-limit wait would count down; only an all-locked pool produces a fixed value forever.

Make the lock a deadline rather than a flag. locked becomes a property that expires on its own, so every existing reader — is_limited, _key_is_unavailable, time_to_key_ready — picks up the recovery with no change. The ceiling is twice the API's request timeout, floored at 60s: generous on purpose, because releasing a key still in use would put two requests on one credential, which is the thing key_concurrency: false exists to prevent, and a streamed response can outlive the request timeout since that bounds only the upstream call. An expiry logs a warning: it means a release was missed, which is a bug worth seeing rather than silently absorbing.

Also stop watchfiles logging every filesystem event at INFO. Uvicorn's reloader watches the working directory, which is where log_file usually lives, so writing a log line produced an event that logged a line — a feedback loop visible in the same report as "1 change detected" every 350ms. That was a regression from routing the standard library through the shared handler, which put those records into the log file for the first time.

Verified by mutation: with the expiry removed, both new tests fail with exactly the production signature — time_to_key_ready pinned at 1.0.

With key_concurrency: false a key is locked for the duration of a
request, and nothing ever expired that lock. Any path that failed to
release one took the credential out of rotation for the lifetime of the
process; once every key had leaked, the API stopped serving entirely and
stayed that way.

The reported symptom was a queue stuck for 30 minutes logging a wait
that never changed:

  DEBUG | nya.core.queue - Resource Unavailable for novelai endpoint - wait time: 0.50s

That constant is diagnostic. time_to_key_ready treats a locked key as
inf, and falls through to a hardcoded `return 1.0` when every key is
inf, which the queue halves to 0.50s. A rate-limit wait would count
down; only an all-locked pool produces a fixed value forever.

Make the lock a deadline rather than a flag. `locked` becomes a property
that expires on its own, so every existing reader — is_limited,
_key_is_unavailable, time_to_key_ready — picks up the recovery with no
change. The ceiling is twice the API's request timeout, floored at 60s:
generous on purpose, because releasing a key still in use would put two
requests on one credential, which is the thing key_concurrency: false
exists to prevent, and a streamed response can outlive the request
timeout since that bounds only the upstream call. An expiry logs a
warning: it means a release was missed, which is a bug worth seeing
rather than silently absorbing.

Also stop watchfiles logging every filesystem event at INFO. Uvicorn's
reloader watches the working directory, which is where log_file usually
lives, so writing a log line produced an event that logged a line — a
feedback loop visible in the same report as "1 change detected" every
350ms. That was a regression from routing the standard library through
the shared handler, which put those records into the log file for the
first time.

Verified by mutation: with the expiry removed, both new tests fail with
exactly the production signature — time_to_key_ready pinned at 1.0.
@k3scat
k3scat merged commit fc50db9 into dev Jul 20, 2026
8 checks passed
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
nya/core/control.py 70.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant