Skip to content

OpenConceptLab/ocl_online#275 | Capacity limit on heavy calls (semantic $match, $rerank), in shadow mode - #922

Merged
paynejd merged 8 commits into
masterfrom
ocl_online-275-capacity-limit
Oct 1, 2026
Merged

paynejd merged 8 commits into
masterfrom
ocl_online-275-capacity-limit

Conversation

@paynejd

@paynejd paynejd commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Part of OpenConceptLab/ocl_online#275: a capacity limit on heavy calls, shipping in shadow mode. It counts and logs how many heavy calls run at once and refuses nothing.

A capacity limit caps how many heavy requests run at once across all users, to protect the API workers and Elasticsearch. It isn't a cap (what an account can have) or a quota (what it can use per period). It charges no quota, and once enforced, a refused call is asked to retry shortly.

What counts as heavy

  • $match with semantic=true (kNN search, bridge calls included) or reranker=true (the in-request rerank).
  • $rerank.

Lexical $match without the reranker isn't gated. The gate sits after authentication, permissions and throttling, and before the match-operations quota is charged.

Lanes

Each lane is a Redis sorted set of leases, with the score set to the lease's expiry by Redis's own clock. One Lua script takes a lease in every lane that applies, atomically. A background thread renews the leases every 20 s, all or nothing (an expired lease is never revived), and they're released in finally. A lease that isn't renewed expires after 60 s, so a worker that dies frees its slots. A lane's TTL is only ever extended.

Lane Starting limit
api_heavy, cluster-wide 4, of which 1 is kept for single-row $match calls (TBv3 search, the Mapper's row panel)
api_heavy_task, per API task 2
es2_knn, semantic $match only 3
Tier ceiling staff 4, core 4, early_access 3, preview 2
Per user staff 4, core 3, early_access 2, preview 1

A user's tier is their highest plan: staff > core_user > early_access > preview (the default).

Modes

  • off: nothing is counted and no headers are sent.
  • shadow (the default): every call goes ahead. A call that finds a lane full is marked shadow-refused in its log line and headers.
  • enforce: a call that finds a lane full gets 429 with Retry-After and {"error_code": "capacity_exceeded", "scope": <lane>, "paused": bool, "retry_after": N}. It's refused before any work or quota charge, so refused calls aren't metered.
    • Retry-After is 5 s per call ahead in the fullest lane, up to 30 s, or 120 s when a lane's limit is 0 (paused).
    • enforce_for=aware (the default) refuses only clients that send "capacity_aware": "true" in X-OCL-Event-Metadata. Other clients (older Mapper tabs, TBv3, scripts) stay in shadow mode.

Headers

Gated responses carry the headers below, and CORS exposes them to browsers:

Header Meaning
X-OCL-Capacity-Decision admitted, shadow-refused, refused or unavailable
X-OCL-Capacity-Limit, -In-Flight The cluster-wide limit that applies to this call, and calls in flight
X-OCL-Capacity-Tier, -Tier-Limit, -Tier-In-Flight The same for the caller's tier
X-OCL-Capacity-Suggested-Concurrency How many calls this user should keep in flight now: 0 while a lane is paused, else at least 1

Runtime config

The mode and every number can be changed at runtime by staff, with no deploy or restart:

  • GET / PATCH / PUT /capacity/config/, GET /capacity/config/history/ and GET /capacity/status/ (live lanes), all IsAdminUser.
  • python manage.py capacity show|set|reset|history|status, for example capacity set tiers.preview=1 --note "...".

Each change adds a CapacityConfig row recording who made it, when, and the old and new config, and writes one ocl_capacity_config log line. Each API process re-reads the config at most every 10 s (CAPACITY_CONFIG_CACHE_SECONDS). CAPACITY_LIMIT_MODE (default shadow) sets the mode only until staff first change it.

Fail open

If Redis can't be reached, calls go ahead uncounted, marked unavailable with the error.

  • The limiter has its own Redis client, with a 0.5 s socket timeout and no retries.
  • Every operation runs on a per-process daemon Redis thread with a short queue and an overall deadline, CAPACITY_REDIS_DEADLINE_SECONDS (1 s), which covers DNS and Sentinel discovery. A hung call never holds up a worker's exit, and a full queue fails at once.
  • After an error, each API process skips Redis for 15 s for acquire, renewal and release; that's less than lease minus two renewals, so one error can't cost a long call its lease. Leases it can't release expire.
  • So an outage delays at most one call per process every 15 s, by at most the deadline.
  • An acquire that Redis runs after its caller gave up (it sat in a buffer while Redis hung) takes no lease.
  • An error inside the limiter never refuses a call, and a config that can't be re-read never refuses either: enforce falls back to shadow. The config re-read runs with a 500 ms statement timeout (CAPACITY_CONFIG_READ_TIMEOUT_MS), so a locked table can't hold a call.
  • Log lines go through a bounded queue and a writer thread, so a backed-up log sink can't hold a call. When the queue is full, the line is dropped and the next one carries log_dropped. At exit, a process waits up to 2 s for lines still queued or being written.

Measurement

Every gated call writes one JSON line ("event": "ocl_capacity") when it finishes. The line has the decision, mode, endpoint, rows, tier, the full lanes, each lane's in-flight count and limit, suggested concurrency, retry_after, held_ms, queue_ms, the request source, algorithm_id, automatch_run_id and cid.

  • queue_ms is how long the call waited between the load balancer and the view, read from X-Amzn-Trace-Id, to whole-second resolution.
  • The metric filters, dashboard and alarms built on these lines are tracked on the ticket.
  • The analytics middleware already forwards response headers, so the capacity headers also reach api_transactions.

Also in this PR

  • get_event_metadata(request) in core/common/utils.py, shared with $match's usage attribution.
  • fakeredis[lua] and lupa in requirements.txt. CI's test job has no Redis, and these run the real Lua scripts in tests. The same tests also pass against Redis 7.0 (CAPACITY_TEST_REDIS_URL=redis://…).
  • A new table, capacity_configs (migration capacity/0001_initial).

Tests

  • core.capacity: 75 tests covering config validation and history, every lane, the reserve, all three modes, enforce_for, a paused tier, Retry-After, lease expiry and all-or-nothing renewal, the renewer thread, fail-open with backoff and the Redis deadline, the non-blocking log queue, process exit with a hung Redis call or a blocked log (subprocesses), a locked config table, headers, the log line, the staff views and the command.
  • The $match / $rerank view tests check that an enforce refusal returns 429 before any quota charge or work, and that a match that raises still releases its lease.
  • Full suite, as CI runs it with no Redis: 2,367 tests OK, 95% coverage. The existing $match / $rerank tests exercise the fail-open path.

Integration test with OpenConceptLab/oclmap#86 (overnight 2026-10-01)

Run locally on a production-like stack:

  • 2 API containers × 4 gunicorn workers, behind a round-robin proxy;
  • Redis 7.0 behind 3 Sentinels;
  • the real embedding and rerank models;
  • production builds of the Mapper (this PR's counterpart and today's main) on their own origins.

Results:

  • Today's Mapper and today's API each work with the other side's new version.
  • Off mode returns byte-identical responses to master.
  • Shadow overhead isn't measurable.
  • No lease leaks after Stop, client aborts, killed workers or container restarts.
  • Fail-open holds through a Redis hang and a Sentinel failover.
  • Enforce mode works end to end with oclmap#86 once it sends capacity_aware.

Fixed in this PR from the test and its reviews:

  • 2701b02: no lease from a late-running acquire.
  • 2f399a1: a paused lane is reported as the scope.
  • 040a689: the skip window is 15 s.

Findings that matter before enforcement are listed on OpenConceptLab/ocl_online#340.

Not in this PR

🤖 Generated with Claude Code

https://claude.ai/code/session_01TyAC9YAn5hP3yGSREkSN8m

…ic $match, $rerank), in shadow mode

Counts semantic and reranked $match calls and $rerank calls in flight, in Redis lanes shared by the API
tasks: cluster-wide, per task, semantic kNN, per plan tier and per user, with one cluster slot kept for
single-row $match calls. Each call takes a lease in every lane that applies in one Lua script, renews it
from a background thread and releases it when it finishes; a lease that isn't renewed expires, so a
worker that dies frees its slots.

- Shadow mode (the default) never refuses. Every gated call writes one JSON log line (decision, tier,
  lane, in-flight counts, endpoint, rows, held and queue time) for CloudWatch metric filters.
- Responses carry X-OCL-Capacity-Decision, -Limit, -In-Flight, -Tier, -Tier-Limit, -Tier-In-Flight and
  -Suggested-Concurrency, exposed to browsers through CORS.
- Enforce mode is built and tested, and ships off: a 429 with Retry-After before any work or quota
  charge. With enforce_for=aware, only clients that send capacity_aware in X-OCL-Event-Metadata are
  refused.
- The mode and every number are runtime config: staff GET/PATCH/PUT /capacity/config/ or
  `manage.py capacity`, applied within 10 s, each change kept with who, when, old and new, and logged.
- Fail open: if Redis can't be reached, calls go ahead uncounted and it's logged. The limiter uses its
  own short-timeout Redis client and skips Redis for 30 s after an error.
- Tests run the real Lua scripts with fakeredis (CI has no Redis), and pass against Redis 7.0 too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TyAC9YAn5hP3yGSREkSN8m

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex adversarial review, pass 1 (codex-cli 0.159.2, commit a6431c0)

Posted verbatim; run read-only against git diff origin/master...HEAD.

Significant findings remain, including risks to shadow-mode latency and lease correctness. I ran git diff origin/master...HEAD; no files were modified and no network was used.

  1. High — core/capacity/config.py:155: Config refresh can block a shadow request indefinitely.
    get_config() performs a synchronous database query while holding _cache_lock, with no connection/query deadline. Catching an exception only helps after the query returns.
    Scenario: PostgreSQL stalls when the cache expires. A $rerank call that could otherwise run waits for the config lookup until gunicorn kills its worker. Shadow mode has changed the call’s outcome.
    Fix: Refresh config outside the request path, serving the last snapshot immediately. Bound database connection/query timeouts and avoid making requests wait for a refresh lock. Test a blocked refresh, not just an immediately raised exception.

  2. High — core/capacity/limiter.py:102: Redis timeouts do not bound total fail-open latency.
    socket_connect_timeout does not bound synchronous DNS resolution. Sentinel discovery can also visit multiple Sentinels sequentially, each with its own timeout, before connecting to the master. The “at most one short timeout” claim is incorrect. Renewal and release additionally bypass is_available(), so already-admitted calls keep attempting Redis during the circuit-breaker interval.
    Scenario: Sentinel DNS stalls or several Sentinels are unreachable. Acquire holds a sync worker for seconds or longer; finishing calls can incur both the renewer join timeout and another release/discovery attempt.
    Fix: Put Redis operations behind an overall deadline using a bounded executor, with no unbounded backlog. Explicitly configure Sentinel retries, and make renewal/release respect the breaker while relying on lease expiry for cleanup. Test blocked DNS, multiple unavailable Sentinels, and release during an outage.

  3. High — core/capacity/limiter.py:63 and :76: A shorter lease can expire an entire shared lane containing longer active leases.
    Every acquire/renew unconditionally sets the key TTL to that call’s lease_ms * 2. Active gates retain their original config, so runtime changes produce mixed lease durations.
    Scenario: A call holds a 60-second lease and renews every 20 seconds. Staff change the config to a 5-second lease with 1-second renewal. A short call acquires and finishes just after the old call renews, leaving the shared key with a 10-second TTL. The key disappears before the old call’s next renewal, deleting its still-valid lease and allowing excess concurrency.
    Fix: Derive key expiry from the largest member expiry, or only extend an existing TTL and ensure it covers all members. Add a mixed-config test that advances time between renewals.

  4. Medium — core/capacity/limiter.py:307 and :366: Failure to start the renewal thread prevents release.
    self.renewer is assigned before start(). If start() raises, acquire fails open with a lease already held. During release, stop() calls join() on the unstarted thread, which raises; the shared try then skips RedisLanes.release().
    Scenario: Thread creation fails under resource pressure. The request proceeds, but all its leases remain until expiry. Repeated failures can exhaust enforce-mode capacity.
    Fix: Attempt Redis release in a separate finally, regardless of renewal-thread cleanup. Track whether the thread started successfully. Test Thread.start() failure and verify every lane is cleared.

  5. Medium — core/capacity/logs.py:9: Synchronous flushed logging can stall shadow calls.
    Every gated call performs print(..., flush=True) before returning. The exception guard handles a broken output stream, but cannot handle a blocked one. Config-change logging has the same issue.
    Scenario: Gunicorn’s captured stdout or its log collector backs up. Completed requests block in release(), occupying all sync workers despite Redis being healthy.
    Fix: Send records through a bounded, nonblocking logging queue with a defined overflow policy and a separate writer. Test a blocked sink, not only the existing OSError case.

  6. Medium — core/capacity/config.py:156: Config failures can still produce enforced refusals.
    A failed config read silently returns the last known config, including mode=enforce. Acquire cannot distinguish a healthy snapshot from this failure and may return 429. This conflicts with the stated requirement that limiter failures allow the request through.
    Scenario: Staff turn enforcement off, but a worker cannot refresh its cached enforce config because the database read fails. That worker continues refusing calls using the stale configuration.
    Fix: Return config health alongside the snapshot, and force shadow/fail-open admission after a failed refresh until a successful refresh. Test failure with a cached enforce configuration and a full lane.

  7. Medium — core/capacity/config.py:190: A config update can commit successfully and then return an error without its required log.
    emit() runs after the transaction commits and is unguarded.
    Scenario: Stdout raises BrokenPipeError. The staff PATCH returns 500, although the new version is already active; the required change log is absent. Existing tests mock emit() successfully throughout config saves.
    Fix: Make post-commit logging failure unable to change the API result. For guaranteed eventual change logging, use a transactional outbox tied to the config version. Add a test where emission fails after persistence.

  8. Medium — core/capacity/config.py:116: Validation permits renewal intervals with effectively no safety margin.
    Checking only renew_seconds < lease_seconds accepts, for example, a 5-second lease renewed every 4 seconds. Scheduling delay and Redis discovery/operation latency can exceed the remaining second.
    Scenario: A valid config causes a live call’s leases to expire before renewal. Another acquire removes them and admits additional heavy work. The old gate records lease_lost but continues running uncounted.
    Fix: Require a substantial renewal margin, accounting for the maximum operation deadline—for example, renewal at no more than one-third of the lease duration. Test delayed renewal and acquisition between expiry and renewal.

  9. Low — core/capacity/limiter.py:204: Queue-time parsing mishandles whitespace and trusts client-supplied trace timestamps.
    Field names are not stripped, so Root=...; Self=... ignores Self and falls back to Root. A preserved client Root timestamp can represent trace age rather than arrival at this API, and callers can influence the logged queue time.
    Scenario: A trace started several minutes earlier is logged as several minutes of worker queueing, distorting capacity measurements.
    Fix: Strip field names/values, validate the trace structure, and distinguish trusted load-balancer arrival timestamps from client trace timestamps. Label the value as an estimate when provenance is uncertain. Add whitespace and spoofed-Root tests.

Things checked and found correct:

  • Lua acquisition checks all lanes before inserting any lease; shadow acquisition forces admission. Argument indexing, millisecond units, and Lua truthiness for a returned ZSCORE are correct.
  • Redis 7 supports the script’s TIME usage with effects replication.
  • Normal $match refusal occurs before check_and_consume_capability; $rerank refusal occurs before model construction.
  • Normal and exceptional view exits release through finally; DRF-handled exception responses reach the header mixin.
  • Staff endpoints use IsAdminUser. PostgreSQL config changes take the advisory transaction lock before reading and merging the previous version.
  • Numeric validation rejects booleans, negative values, unknown settings, and malformed nested shapes. PUT’s reset-to-default behavior is documented.
  • Capacity headers and Retry-After are exposed through CORS. The migration adds a new table without rewriting existing application tables.
  • Capacity tests default to fakeredis with Lua support, without requiring a Redis service. I found no added credentials or concrete production host/account identifiers.

Verification limitation: Docker access was denied, and local pylint failed before analysis because the read-only sandbox has no writable temporary directory. Tests, pylint, and the 95% coverage threshold therefore remain unverified.

…ew, pass 1

- A lane's TTL is only ever extended, so a shorter lease (after a runtime change) can't expire a
  lane that holds longer leases.
- If the renewer thread fails to start, the leases are still released: stopping the renewer and
  releasing are separate steps.
- Renewal and release skip Redis while it's marked failed (the leases expire), and release no longer
  waits on a renewal in flight, which can only extend leases the call still holds.
- A config that can't be re-read never refuses: a cached enforce config falls back to shadow until a
  read succeeds.
- A config change that's saved no longer fails the request if its log line can't be written.
- renew_seconds must be at most a third of lease_seconds, and lease_seconds at least 15.
- Sentinel connections are explicitly retry-free; the trace header parser tolerates whitespace.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TyAC9YAn5hP3yGSREkSN8m
@paynejd

paynejd commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Response to Codex pass 1, fixed in 626b666:

# Finding Action
3 A shorter lease can expire a shared lane Fixed. The scripts only ever extend a lane's TTL. Tested on fakeredis and on Redis 7.0
4 Renewer start failure skips the release Fixed. The renewer is kept only once it has started, and stopping it and releasing are separate steps. Tested
6 Stale cached enforce config can refuse Fixed. A config that can't be re-read falls back from enforce to shadow until a read succeeds. Tested
7 Config saved, then the log line fails the request Fixed. The log line is guarded; the saved row is the record. Tested
8 Renewal margin too thin Fixed. renew_seconds must be at most a third of lease_seconds, and lease_seconds at least 15
9 Whitespace in X-Amzn-Trace-Id Fixed. Behind the load balancer, Self is always its own stamp when a client sent the header, and Root is its own when none was sent. The value is only logged
2 Fail-open latency Partly fixed. Renewal and release now skip Redis while the per-process breaker is open (leases expire), release no longer waits on a renewal in flight, and Sentinel connections are explicitly retry-free. Declined: an overall-deadline executor for DNS. The breaker bounds the cost to one call per process per 30 s, and the existing cache/throttling client on the same Redis has the same exposure with more retries
1 Config read can block Declined. It's one primary-key query on a tiny table, at most every 10 s per process. Auth, permissions and the tier lookup use the same database on every call, so a stalled database stalls every request already
5 Flushed print can block Declined. The same stdout already carries several synchronous timing prints per $match row; one more line per call doesn't change that

Capacity tests: 61 pass, on fakeredis and on Redis 7.0.

…ew, pass 2

- Every Redis operation runs on a per-process Redis thread with an overall deadline
  (CAPACITY_REDIS_DEADLINE_SECONDS, 1 s), which also bounds DNS and a walk through the Sentinels. An
  operation still queued when its caller gives up is skipped.
- Log lines go through a bounded queue and a writer thread, so a backed-up log sink can't hold a call;
  when the queue is full a line is dropped, and the next one says how many were.
- Renewal is all or nothing: leases are renewed only if the call still holds every one unexpired, so an
  expired lease is never revived; once a lease is lost the renewer stops.
- An error inside acquire never refuses a call: a call that doesn't hold a lease becomes unavailable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TyAC9YAn5hP3yGSREkSN8m

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex adversarial review, pass 2 (codex-cli 0.159.2, commit 626b666)

Posted verbatim; run read-only against git diff origin/master...HEAD, with pass 1's fixes and declines as context.

  1. High — core/capacity/limiter.py:159–164, 301–305, 364–386: The breaker does not bound outage latency as claimed.
    It opens only after an operation fails; there is no atomic probe reservation. A renewer blocked in Redis can overlap with release or a new request, and DNS resolution has no overall deadline. An indefinitely blocked attempt never opens the breaker. With eight sync workers, this can still exhaust API capacity in shadow mode. Existing Django Redis exposure does not make this additional path bounded.

    Fix: Add a nonblocking, per-process single-flight probe and an overall deadline covering discovery/DNS and Redis execution. Skip counting when either is unavailable. Remove the remaining join(timeout=0.2) response delay. Test overlapping renewal/release and a connection attempt that never returns.

  2. Medium — core/capacity/limiter.py:312–320: An acquisition failure can leave a call refused.
    The exception handler preserves an already assigned decision. If get_retry_after() raises after DECISION_REFUSED is set, the request still returns 429 despite a limiter failure, with no valid Retry-After value. I reproduced this using the actual class with an injected retry-calculation exception: refused=True, retry_after=None.

    Fix: On acquisition exceptions, unconditionally switch to DECISION_UNAVAILABLE and clear refusal metadata while retaining any acquired lease for cleanup. Add a view-level regression test asserting work and quota consumption proceed after this failure.

  3. Medium — core/capacity/limiter.py:79–84: Renewal resurrects expired leases and can renew only part of a call’s lanes.
    ZSCORE checks membership, not whether the score is still in the future. After a delayed renewal, another user’s acquisition can prune this call from shared lanes while its expired user-lane member remains. Renewal then revives that user lease, reports lease_lost, and continues renewing it. The original call is absent from cluster accounting but can keep subsequent calls from that user refused.

    Fix: Check expiry against now_ms, and verify all required leases before renewing any. If a lease is missing or expired, mark the lease lost and stop renewal; do not revive surviving expired members. The current test explicitly expects an expired score of 1000 to be revived, so it needs changing.

  4. Medium — core/capacity/logs.py:9, core/capacity/limiter.py:389–393: Shadow mode still adds an unbounded stdout wait on previously print-free paths.
    Catching exceptions does not bound blocking print(..., flush=True). The declined rationale about existing per-row timing prints does not cover an over-quota $match: previously it returned 403 before search and those prints; now gate cleanup synchronously emits JSON first. A backed-up stdout pipe can hold that response indefinitely.

    Fix: Emit through a bounded, nonblocking logging queue with an explicit overflow policy. Test a blocked sink, rather than only a sink that raises immediately.

Checked and found correct by source inspection:

  • keep() prevents shorter leases from shortening shared-key TTLs.
  • Renewer startup failure now permits release; stop and release failures are handled separately.
  • Failed config refresh changes cached enforce mode to shadow.
  • Config emit exceptions are guarded after persistence.
  • Renewal-margin validation, trace whitespace parsing, staff permissions, PostgreSQL advisory locking, PUT default replacement, and history recording are consistent.
  • Admission precedes capability consumption; refusal takes no lease. CORS exposes the new headers.
  • Lua admission is atomic, uses Redis time, and has consistent argument indexing. Migration is additive; no concrete secrets or infrastructure host identifiers appeared in the diff.

No files were modified or network used. Docker checks were blocked by socket permissions; Django tests and coverage were not run. Limited capacity-package pylint passed with import errors disabled; git diff --check passed.

@paynejd

paynejd commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Response to Codex pass 2, fixed in 570833a:

# Finding Action
1 The breaker doesn't bound outage latency Fixed, reversing the pass-1 decline. Every Redis operation now runs on a per-process Redis thread with an overall deadline (CAPACITY_REDIS_DEADLINE_SECONDS, 1 s), which covers DNS and Sentinel discovery. An operation still queued when its caller gives up is skipped. Release waits at most 0.1 s for a renewal in flight. Tested with a Redis call that hangs, and with a queued call whose caller gave up
2 An error after refused still refuses Fixed. A call that doesn't hold a lease becomes unavailable on any limiter error. Tested at the gate and at the $match view: the match runs and is metered
3 Renewal revives expired leases, or renews only some lanes Fixed. Renewal is all or nothing: it renews only if the call still holds every lease unexpired, and the renewer stops once a lease is lost. Tested on fakeredis and on Redis 7.0
4 Blocking stdout on paths that had no prints Fixed, reversing the pass-1 decline. Log lines go through a bounded queue and a writer thread. When the queue is full, the line is dropped and the next line carries log_dropped. Tested with a blocked sink

Capacity tests: 69 pass, on fakeredis and on Redis 7.0.

…ew, pass 3

- Redis operations run on a daemon thread with a short queue (core/capacity/threads.py) instead of a
  ThreadPoolExecutor, whose threads the interpreter waits for at exit: an operation stuck in DNS no
  longer holds up a gunicorn worker's recycle. When the queue is full, a call fails at once.
- The log sink tracks lines in flight, and at exit waits for them for 2 s at most, so the last line
  (e.g. `manage.py capacity set`'s change) isn't lost, and a blocked sink can't hold the exit. The
  dropped-line count is kept under a lock.
- The config refresh on the request path runs with a 500 ms statement timeout
  (CAPACITY_CONFIG_READ_TIMEOUT_MS), so a locked or stalled table can't hold a call; the last known
  config then applies, with enforce falling back to shadow.
- Tests: process exit with a hung Redis call and with a blocked log sink (subprocesses), the last line
  printed at exit, concurrent emitters, a full Redis queue, and a locked config table.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TyAC9YAn5hP3yGSREkSN8m

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex adversarial review, pass 3 (codex-cli 0.159.2, commit 570833a)

Posted verbatim; run read-only against git diff origin/master...HEAD, with pass 2's fixes as context.

Significant findings remain in 570833af.

  1. High — core/capacity/config.py:151–164: config refresh can block requests indefinitely, including shadow requests.

    get_current() runs synchronously while holding _cache_lock. Neither the database connection nor this query has a limiter-specific deadline. The exception fallback only helps once the database returns or raises; the Redis executor deadline does not cover this path.

    Failure scenario: a transaction holds an exclusive lock on capacity_configs. As each worker’s cache expires, its next heavy request blocks before matching or reranking. All eight workers can become occupied by the limiter, even though the tables needed by the original requests remain available. Threaded callers additionally wait indefinitely on _cache_lock.

    Fix: refresh config outside the request path using a bounded background refresh and appropriately bounded database connection/query timeouts. Requests should immediately use the last valid config, falling back from enforce to shadow when freshness cannot be confirmed. Add a regression test with a stalled refresh, rather than only an immediately raised database exception.

  2. High — core/capacity/limiter.py:159,176–182: a timed-out Redis operation can prevent a gunicorn worker from exiting.

    ThreadPoolExecutor workers participate in Python’s interpreter shutdown join. Timing out future.result() abandons the caller’s wait; it does not stop the running operation or remove that shutdown dependency.

    Failure scenario: DNS resolution remains stuck after the request’s one-second deadline. Requests subsequently fail open, but when the worker reaches --max-requests, interpreter shutdown waits for the executor thread. The worker stops serving and remains alive until gunicorn forcibly kills it. With the configured 600-second timeout, recycling can leave worker capacity unavailable for minutes.

    I reproduced this shutdown behavior in a subprocess: the main thread finished, but the process remained alive waiting for the executor.

    Fix: use a dedicated daemon worker with a bounded submission queue and no interpreter-exit join dependency. While an operation remains stuck, keep that executor unavailable instead of repeatedly queueing abandoned probes. Add a subprocess test proving that process exit completes while an operation remains blocked; the current finite-sleep tests cannot catch this.

  3. Medium — core/capacity/logs.py:45–47,58–66: exit draining can lose the final log line or block shutdown indefinitely.

    The writer removes a line from the queue before writing it. drain() only sees queued lines and does not coordinate with the writer’s in-flight line. It also writes synchronously without a deadline.

    Failure scenario: manage.py capacity set queues its change record; the daemon writer dequeues it but has not finished writing when the command exits. drain() sees an empty queue, returns, and interpreter shutdown terminates the writer, losing the config-change log. Conversely, with queued records and blocked stdout, drain() itself can hang during worker recycling.

    I reproduced the first race using the actual module: drain() returned with an empty queue and no written record while the writer held the final line. The drain test patches out the writer, so it misses this case.

    Fix: track in-flight writes and implement coordinated shutdown with a strict deadline. Give the writer bounded time to finish both queued and in-flight records; avoid unbounded synchronous writes in atexit. Test both an in-flight final record and a blocked sink during process exit.

  4. Low — core/capacity/logs.py:23–32: concurrent emitters corrupt dropped-line accounting.

    Reading dropped, enqueueing the associated record, and subtracting that count are not atomic.

    Failure scenario: two emitters both read dropped=3, successfully enqueue records reporting three lost lines, then each subtracts three. The counter becomes -3, and subsequent records can report negative loss. I reproduced that interleaving. Sync gunicorn limits its production likelihood, but the module is unsafe for concurrent callers.

    Fix: serialize dropped-counter bookkeeping and the nonblocking enqueue under a short lock; leave stdout writes outside it. Add a coordinated concurrent-emitter test.

Things checked and found correct by inspection:

  • Pass 2’s acquire-error fix clears refusal when acquisition fails without a held lease.
  • Renewal checks every lease for unexpired ownership before extending any; the renewer stops after detecting lease loss.
  • All Redis I/O entry points use RedisLanes.call(). Queued abandoned operations skip execution.
  • With no preload, executor initialization happens within workers. I found no lock cycle between ordinary executor submission and Redis client initialization.
  • Enforced refusal precedes $match quota consumption and reranking work. The extracted match() preserves the existing charge/refund logic.
  • Normal and exceptional view exits release through finally; handled DRF responses receive headers through finalize_response.
  • Lua argument indexing, expiry comparisons, TTL extension, and shared single-row reserve logic are consistent.
  • Staff endpoints use IsAdminUser; PostgreSQL advisory locking serializes config updates; the migration is additive.
  • Capacity response headers are exposed through CORS. I found no new credentials or concrete infrastructure addresses in the change.

Changed-module pylint passed at 10.00/10, and git diff --check passed. I did not run database-writing tests or full CI coverage during this read-only review. No files were modified and no network was used.

@paynejd

paynejd commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Response to Codex pass 3, fixed in 4be4acf:

# Finding Action
1 Config refresh can block a call on the database Fixed, reversing the earlier decline. The refresh on the request path runs with SET LOCAL statement_timeout (CAPACITY_CONFIG_READ_TIMEOUT_MS, 500 ms), which covers lock waits. Then the last known config applies, with enforce falling back to shadow. It's skipped inside a transaction, where the timeout would outlive the read; requests don't run in one. Tested with the table locked from a second connection
2 A timed-out Redis call can hold up a worker's exit Fixed. Operations run on a daemon thread with a short queue (core/capacity/threads.py), and a full queue fails at once. Tested in a subprocess: a process with a hung call exits at once, where a ThreadPoolExecutor holds exit for the whole hang
3 Exit drain can lose the last line, or block Fixed. The sink counts lines queued or in flight, and at exit it waits for them for 2 s at most. Tested in subprocesses: the last line is printed, and a blocked stdout doesn't hold the exit
4 Dropped-line accounting races Fixed. The count is kept under the sink's lock. Tested with 4 threads emitting into a full queue

Capacity tests: 75 pass, on fakeredis and on Redis 7.0.

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex adversarial review, pass 4 (codex-cli 0.159.2, commit 4be4acf)

Posted verbatim; run read-only against git diff origin/master...HEAD, with pass 3's fixes as context.

  1. Medium — core/capacity/config.py:153–169: config refresh still has no end-to-end deadline. SET LOCAL statement_timeout bounds execution on PostgreSQL; it does not bound waiting for connection establishment, sending that statement, receiving results, or completing the transaction. All this runs synchronously in the request thread.

    Failure scenario: a staff $rerank request finishes authentication, then PostgreSQL connectivity stalls during a failover just as its config cache expires. The new config read can occupy the sync worker until TCP recovery or gunicorn’s 600-second timeout, although reranking could otherwise proceed without another database query. The fallback never runs while the database operation remains blocked. The table-lock test at core/capacity/tests/tests.py:1107 cannot detect this—it uses a responsive server.

    Fix: refresh config using a bounded daemon reader that owns a separate Django database connection. Requests should use the cached config immediately, downgrading enforce to shadow when freshness cannot be confirmed. Keep the SQL timeout, add connection timeouts, and test a reader that never returns.

Things checked and found correct:

  • Pass 3’s Redis daemon, bounded queue, cancellation, and caller deadline fixes are present.
  • The log sink tracks pending writes under its condition and bounds its exit drain.
  • Config refresh correctly bounds PostgreSQL table-lock waits and downgrades enforce after read failures.
  • Lua admission is atomic; Redis 7 time handling, expiry comparisons, and TTL extension look correct.
  • Renewal cannot recreate released leases; release runs in finally; stale leases expire.
  • Capacity refusals precede $match quota consumption. Shadow refusals do not add Retry-After.
  • Staff permissions, PATCH/PUT semantics, advisory-lock serialization, history, CORS headers, and the additive migration look correct.
  • No newly committed secrets or concrete infrastructure identifiers found.

Targeted pylint passed 10.00/10. Tests and coverage were not run because they modify the test database. No files were modified and no network access was used.

@paynejd

paynejd commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Response to Codex pass 4. Declined, with no code change:

# Finding Action
1 The config refresh has no end-to-end deadline if a database connection stalls Declined. The refresh is bounded against lock waits and slow queries (statement_timeout). What's left is a connection that stalls between authentication and this read, which are milliseconds apart on the same connection. The gate's tier lookup queries the same database on every call too. A database that stalls stalls every request at authentication already, so the limiter adds no new outage mode there. A background reader with its own connection would add a database connection per worker to close that window. Listed as a residual risk below

Residual risks, for the merge decision:

  • DNS. Bounded at 1 s per Redis operation by the deadline. A call that's still stuck finishes in the background, and a late acquire's lease expires unrenewed.
  • Database stall inside a heavy call (above). Same exposure as the rest of the request.
  • queue_ms resolution. Whole seconds, from the load balancer's trace header. It's a measurement only.

Codex found nothing further in passes 1–4 that this PR leaves open. Full suite locally, as CI runs it: 2,385 tests OK, 95% coverage.

paynejd and others added 4 commits September 30, 2026 23:51
… too late takes no lease

Found in a local soak with Redis behind Sentinel: while Redis hung (docker pause), acquire scripts that
had timed out sat in the socket buffers and ran when Redis came back, taking leases nobody held. For up
to lease_seconds after recovery the counts read high (in enforce mode that would refuse calls). The
acquire now carries a not-after time (the caller's deadline plus 2 s for clock differences) and does
nothing if Redis runs it later.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TyAC9YAn5hP3yGSREkSN8m
…the paused lane when one is (Codex review of #922 + oclmap#86 together)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TyAC9YAn5hP3yGSREkSN8m
…fter an error, not 30, so one error can't cost a long call its lease (independent review of #922 + oclmap#86)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TyAC9YAn5hP3yGSREkSN8m

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex adversarial review across both PRs (codex-cli 0.159.2): #922 at 2701b02 + OpenConceptLab/oclmap#86 at 7519657

From the overnight integration test of the two PRs as a set (OpenConceptLab/ocl_online#275). Focus: the contract between them. Posted verbatim; read-only. Since then: finding 1 (the capacity_aware flag) is fixed in oclmap#86 43649ce + 9d1bb52, and finding 5 in oclapi2#922 2f399a1. Findings 2–4 are listed for OpenConceptLab/ocl_online#340, before enforcement.

Significant issues remain. The planned shadow rollout is compatible, but switching only runtime config to enforcement will not enforce against this Mapper under the default enforce_for=aware.

  1. High — Enforcement prerequisite missing.
    oclmap:src/services/attribution.js:64; src/components/map-projects/MapProject.jsx:3811
    No capacity_aware occurs anywhere in Mapper’s src. The attribution builder omits it, and the interactive single-row $match path sends no attribution metadata at all.

    Failure: changing the API to mode=enforce, enforce_for=aware leaves all these Mapper calls effectively in shadow: they acquire leases even when lanes are full. Switching to all activates refusals, including for old tabs that cannot handle them safely.

    Fix: before #340 activation, merge "capacity_aware":"true" into metadata for every retry-capable heavy OCL call: bulk and interactive semantic $match, $match with inline reranking, and standalone $rerank. Preserve existing metadata; use a JSON string, not boolean true. Do not rely solely on modifying the builder—the interactive path must actually send its headers. This is an activation blocker, not a blocker for shipping shadow mode.

  2. High — Ambiguous failures can duplicate heavy work and consumption.
    oclmap:src/services/capacity.js:37,260; oclapi2:core/concepts/views.py:1223
    Mapper retries network failures and 502/503/504 twice, without request deduplication. An 11-minute browser timeout does not guarantee that every earlier gateway failure means the original operation stopped.

    Failure: an ALB/gateway returns 504 or the connection breaks while Django continues searching. Mapper sends another $match; both execute and consume operations if successful. During Redis failure, both are admitted uncounted. Even when Gunicorn actually kills the worker at 600 seconds, the precharged operations have no guaranteed refund because process death bypasses the exception handler.

    Fix: add server-backed idempotency/deduplication for $match, including consumption and result recovery; otherwise avoid automatic retries after ambiguous execution failures. Validate the actual ALB timeout alongside Gunicorn’s timeout. The ALB configuration was unavailable locally.

  3. Medium — The “per-tab” gate and rerank limit reset on project unmount.
    oclmap:src/components/map-projects/MapProject.jsx:236,361,1804
    Gates and the two-slot rerank limiter are component refs. Unmount does not cancel capacity waits or invalidate the run ticket; cancellation checks only Stop and ticket changes in that component.

    Failure: start work in project A, navigate to project B, then start another run. A’s waits and outstanding calls can continue, while B creates an independent gate and two more rerank slots. One tab can exceed two concurrent $rerank calls and ignore A’s server pause.

    Fix: keep server gates and the rerank limiter in a tab-scoped service. On unmount, cancel queued work and waits through a component/run cancellation token. Outstanding server work may finish, but must retain its shared limiter slot until settlement.

  4. Medium — Capacity refusals consume DRF rate allowance; Mapper conflates the two 429s.
    oclapi2:core/concepts/views.py:952,1213; oclmap:src/services/capacity.js:242
    DRF throttle checks run before the view’s capacity gate. A request subsequently refused for capacity has already passed—and consumed—the rate allowance. Mapper treats every 429 identically, without inspecting error_code, scope, or rate-limit headers.

    Failure: at a five-second capacity delay, one waiting request can make roughly 288 attempts in 30 minutes with average jitter. Repeated runs or parallel calls can burn substantial portions of the standard 5,000/day $match allowance without matching anything. Eventually a DRF daily throttle replaces capacity refusals; Mapper still describes the condition as capacity throttling. A minute throttle also pauses unrelated heavy endpoints sharing the server gate.

    Fix: ensure capacity-refused calls do not retain DRF allowance consumption, using a concurrency-safe design. Classify error_code === "capacity_exceeded" separately from DRF throttles; retain bounded waiting for both, but show the correct reason and apply appropriately scoped pauses.

  5. Low — paused can contradict the reason for the long retry delay.
    oclapi2:core/capacity/limiter.py:405,460
    Retry delay considers any full lane with limit zero, but scope and paused describe only the first full lane.

    Failure: the cluster lane is full and the preview tier is paused at zero. The response uses the paused delay of 120 seconds but reports scope:"api_heavy", paused:false. Mapper currently ignores these fields, so waiting works, but consumers cannot reliably distinguish a configured pause from temporary congestion.

    Fix: prioritize a zero-limit lane as the reported scope, or define paused as “any applicable lane is paused” and expose all blocking scopes.

What I checked and found correct:

  • Headers/CORS: all seven emitted names match Mapper’s case-insensitive parser. Decision/tier are strings; counts, limits and suggested concurrency parse as numbers. Retry-After and all seven capacity headers are explicitly exposed through CORS; X-OCL-Event-Metadata is allowed for preflight. Mapper’s optional suggested-spacing header is absent on this API and safely omitted.
  • Response coverage: gated 200s, capacity 429s and DRF-handled errors after acquisition receive capacity headers. Errors before acquisition—authentication, DRF throttling, $rerank input validation—do not. Off mode, ungated calls and preflight have no capacity decision headers. Proxy-generated errors cannot receive Django’s headers.
  • 429 shape: detail, error_code, and scope are strings; paused is boolean; retry_after is an integer in seconds. Capacity refusal precedes capability consumption. DRF throttles have a different body and ordinarily no capacity headers.
  • Retry parsing: numeric seconds and HTTP dates work; zero/past dates get a one-second minimum wait. Missing headers fall back to numeric body seconds, then exponential delays of 5–60 seconds. An invalid present header bypasses body fallback. Huge delays immediately end as throttled without poisoning the shared gate. Header-driven waits receive 0–50% jitter.
  • Bounds and Stop: Auto Match accumulates up to 30 minutes of waiting, not total elapsed execution time. Single-row $match uses five minutes; interactive $rerank currently uses the default 30 minutes. Stop interrupts configured waits within a 250-ms poll and prevents further run retries; already executing server work continues.
  • Leases: 60 seconds is a renewable lease, not a request duration limit. Healthy 20-second renewal keeps a long call counted. Renewal is atomic across all lanes and never revives expired leases. With default settings, Gunicorn kills an overlong sync request at roughly 600 seconds, before the 900-second renewal ceiling; its count then expires within 60 seconds of the last renewal. If a call survives beyond the renewal ceiling, or renewal is lost during a Redis outage, it eventually continues uncounted. Fail-open therefore provides availability, not a hard concurrency guarantee.
  • Mixed versions: old Mapper + new shadow API remains compatible. Old tabs + aware enforcement remain admitted; old tabs + all can hang on their legacy 429 path. New Mapper + old API tolerates absent capacity headers and handles ordinary DRF 429s. During a mixed API rollout, old tasks perform uncounted work, so cluster limits cannot be authoritative until every task is upgraded. New workers also converge on runtime config over the cache interval, rather than instantly.
  • Verification: confirmed both requested HEADs and ran 130 focused Mapper tests: all passed. API behavior was reviewed from source and existing tests; no live Django/Redis/ALB verification was performed. No files were modified and no network was used.

@paynejd

paynejd commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

Independent Claude review across both PRs (a read-only Claude subagent, run in the overnight integration test of #922 + OpenConceptLab/oclmap#86, OpenConceptLab/ocl_online#275). Reviewed: oclapi2#922 at 2701b02, oclmap#86 at 7519657. Posted verbatim. Since then: finding 1 is fixed in oclmap#86 (43649ce, 9d1bb52), and finding 7 in oclapi2#922 (040a689: skip window 15 s). Findings 2–6 are listed for OpenConceptLab/ocl_online#340, before enforcement.

Cross-repo contract review: oclapi2 PR 922 and oclmap PR 86

I only read code. No files were changed and no APIs were called. Nothing here breaks the shadow-mode deploy. Findings 1–3 must be fixed before enforcement is turned on.

Findings

1. HIGH: the Mapper never sends capacity_aware, so enforcement can't reach it.

  • Where: oclmap src/services/attribution.js:52-86 (searched all of src/, no capacity_aware anywhere); oclapi2 core/capacity/limiter.py:398-403 and config.py:26.
  • Under enforce_for=aware (the default), Mapper calls are never refused. The cap never applies to the main heavy-call client.
  • Under enforce_for=all, old Mapper tabs break:
    • On origin/main, $rerank goes through service.post. On a 429, sendRequest returns a promise that never settles and opens the full-screen throttle dialog, so runs freeze.
    • Old $match batches treat a 429 as not retryable and mark the rows failed (-2).
  • So right now neither setting is usable.
  • Fix: send "capacity_aware": "true" (a string, per the metadata convention) only on gated calls:
    • the bulk batch $match (MapProject.jsx:2309)
    • the single-row $match (MapProject.jsx:3811-3833). It currently sends no X-OCL-Event-Metadata at all, so the header has to be added.
    • the bridge $match (merge it into the component's headers in getBridgeMatchService, :2101)
    • $rerank (:4373)
  • Don't send it on ScispaCy, logs or lookups. Also add the key to event-metadata-vocabulary.md.

2. HIGH (only once enforced): the Mapper runs more calls at once than the server allows per user, and the retrying batch starves.

  • Where: oclmap requestLimits.js:20-22 and autoMatchRows.js RERANK_MAX_IN_FLIGHT=2, against oclapi2 config.py:34 (per_user: preview 1, early access 2, core 3).
  • A preview user runs 2 $match batches plus 2 $rerank calls against a limit of 1. Early access runs 5+2 against 2.
  • On a 429, capacity.js:248-256 pauses the gate for exactly Retry-After, but the refused request sleeps Retry-After × 1–1.5.
  • runWithConcurrency (matchBatch.js:134-147) sends the next queued batch the moment the gate reopens, with no jitter. So a new batch always takes the freed slot first.
  • Failure scenario: early-access run, 3 batches stuck retrying.
    • Each gets a 429 every ~6 s until the queue drains, or until its 30-minute cap runs out.
    • At the cap the batch ends throttled. isRunThrottled(algo.id) then marks every remaining row throttled, even though the server was serving the user the whole time.
  • Server side: a refused call takes no lease, so a full lane's count never goes above its limit. ahead (limiter.py:409) is therefore always 1 and Retry-After is always 5 s, however many clients are waiting.
  • Fix, before enforcing:
    • Cap heavy calls in flight per tab at X-OCL-Capacity-Suggested-Concurrency (ocl_online#340), or line the per_user defaults up with the Mapper's caps.
    • Let a refused request retry ahead of new sends: the scheduler should wait the pause plus the maximum jitter, or use a FIFO slot per server.

3. MEDIUM: a capacity 429 and a DRF throttle 429 look the same to the Mapper, and capacity retries use up DRF quota.

  • Where: capacity.js:242 (any 429), APIService.js:172.
  • $match uses DRF throttles (views.py:952; match_standard_day is 5000/day).
  • DRF counts each attempt in initial(), before the gate in post(). Every capacity-refused retry therefore uses one daily hit: about 1,700 per hour with three starving batches (finding 2).
  • When the daily limit is reached, the Retry-After is hours long, which is over the cap. Every batch then ends "throttled — server busy" at once.
  • handlesThrottle also hides the old countdown dialog, so the user is told the server is busy when they actually hit a rate limit.
  • Fix:
    • Mapper: branch on data.error_code === 'capacity_exceeded', and show any other 429 as a rate limit.
    • Server: don't count calls the capacity limit refuses against the DRF throttles (gate before the throttle check, or refund the hit).

4. MEDIUM: heavy calls are re-sent after long failures.

  • Where: capacity.js:32-40, retry.js:6-10.
  • When gunicorn kills a worker at 600 s, the ALB returns a 502. The Mapper re-sends twice more, which can mean up to 3 × 600 s of worker time.
  • Each attempt is charged match units, and nothing is refunded when the worker is killed.
  • The 11-minute axios timeout doesn't count time spent queued at the ALB or in the backlog. The resulting ECONNABORTED (no response) is treated as transient, so the call is re-sent while the server is still working on it. An ALB idle timeout under 11 minutes causes the same with 504s.
  • Fix: retry 502/504/timeouts only when the attempt failed quickly (say under 30 s, which is the deploy/restart case), or send an idempotency key.

5. MEDIUM-LOW: one gate per server ignores scope.

  • Where: MapProject.jsx:1749-1760, capacity.js:213-233.
  • A bulk batch's user or es2_knn 429 also holds back:
    • lexical $match, which the server doesn't gate
    • the row panel's single-row $match, which the server reserves a cluster slot for (reserve_single_row)
    • $rerank
  • Interactive calls end up waiting behind bulk pauses.
  • Fix: pause by the 429 body's scope, or let single-row calls skip the gate's pause.

6. LOW: a manual $rerank waits up to 30 minutes and can't be stopped, while holding a rerank slot.

  • Where: MapProject.jsx:4261, 4351, 4370-4378. There's no maxWaitMs, and isCancelled is always false when it isn't run traffic.
  • Two manual reranks waiting block every run rerank for up to 30 minutes.
  • Fix: use INTERACTIVE_WAIT_CAP_MS for manual reranks, and wait on the gate before taking a limiter slot.

7. LOW: a single Redis error can lose a long call's lease.

  • Where: limiter.py:197-203, 424-431, CAPACITY_REDIS_RETRY_SECONDS=30.
  • A renewal fails at T+20, so the process skips Redis until about T+51 and the T+40 renewal is skipped. The T+60 renewal arrives at expiry, the lease is lost, and the renewer exits.
  • The call goes uncounted for its remaining minutes. This fails open, but it skews the counts.
  • Fix: make the skip window shorter than lease − 2×renew, or let renewals retry sooner.

8. NIT: the Mapper parses x-ocl-capacity-suggested-spacing-ms (capacity.js:88). The server neither sends nor exposes it.

Checked and found correct

  • Header names: all 7 X-OCL-Capacity-* headers plus Retry-After are in CORS_EXPOSE_HEADERS. axios 1.x header lookup ignores case, and the Mapper's lowercase names resolve.
  • Which responses carry them: every gated response, including 4xx/5xx raised inside the gate (finalize_response). DRF throttle 429s, ungated calls and mode=off carry none. Retry-After is only set on real refusals.
  • 429 body: {detail: str, error_code: "capacity_exceeded", scope: str, paused: bool, retry_after: int}. The refusal comes before any quota charge.
  • Retry-After parsing: 0 waits at least 1 s; missing waits 5 s, doubling to 60 s; HTTP-dates work; negative values fall back to the default; very large values end the request throttled without pausing the gate; the body's retry_after is used when the header is missing.
  • No gated Mapper path still uses the never-settling sendRequest: bulk, single-row and bridge $match and $rerank all go through request + handlesThrottle + requestWithCapacityRetry. A 429 ends rows -4 (throttled), not -2 (failed). A 500 is an error; 502/503/504 get bounded retries.
  • Stop: checked every 250 ms in waits, in the scheduler (untilCancelled) and in the limiter queue.
  • Fail-open: Redis errors mean no 429s, so the Mapper can't get into a retry storm. Enforce falls back to shadow if the config can't be read.
  • Leases vs the 600 s gunicorn timeout: a killed worker's leases free within 60 s, max_hold (900 s) never applies, and renew/release run in order on the one Redis thread.
  • Mixed versions:
    • Old Mapper + new API in shadow: no change in behaviour.
    • New Mapper + old API: works with no capacity headers.
    • Old and new API tasks mid-deploy: only the counts are low for that window (old tasks don't count). Config compatibility holds both ways (merge(known_only)), and a missing migration falls back to the defaults.

@paynejd

paynejd commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

Overnight integration test: #922 + OpenConceptLab/oclmap#86 (2026-10-01)

The two PRs were tested together on a local production-like stack:

  • 2 API containers with 4 gunicorn workers each, behind a round-robin proxy;
  • Redis behind Sentinel;
  • the real embedding and rerank models;
  • production builds of this Mapper and of today's main, on their own origins, with headless browsers per user.

No merges or deploys. Every decisive row was re-run at the final heads (oclapi2#922 c5282f4, oclmap#86 9d1bb52; CI green on both).

Compatibility (decides the deploy order): both directions work.

  • This Mapper with today's API: Auto Match results are the same as with the new API (25-row and 100-row two-algorithm runs). Saved projects, closed run records, no alerts.
  • Today's Mapper with the new API in shadow mode: results identical to today's API, nothing refused, capacity headers present, CORS correct.
  • A rolling API deploy (old → new, one container at a time) during a run of today's Mapper: run completed, same results.

The new API in shadow mode:

  • Off mode returns identical responses and quota usage to master.
  • Overhead isn't measurable at this load.
  • No lease leaks after Stop, client aborts, a killed worker or a container restart.
  • Workers recycle cleanly.
  • The runtime off switch takes effect within the 10 s config cache under traffic.
  • Fail-open holds through a Redis hang and an unclean master loss: runs finish with the same results. Calls during the outage are uncounted.

This Mapper against real capacity 429s (enforce mode, which ships off):

  • Runs wait out refusals and finish with the same results and no failed rows, including with 4 users at once.
  • Stop works during a wait; a paused tier shows "Waiting for capacity".
  • With both APIs down, rows in flight end as "Matching failed … Run Auto Match on those rows again", never as "no candidates".

Commits added overnight:

  • oclmap#86:
    • 43649ce sends "capacity_aware": "true" on OCL's gated calls. Without it, enforcement could never reach this Mapper.
    • 9d1bb52 adds one tested rule for which calls go to OCL.
    • Codex reviewed both: two findings fixed, then clean.
  • oclapi2#922:
    • 2701b02: an acquire that runs after its caller gave up takes no lease (found by this test).
    • 2f399a1: the 429 scope names the paused lane.
    • 040a689: a 15 s Redis skip window, so one error can't cost a long call its lease.

Before enforcement (OpenConceptLab/ocl_online#340), from the test and the reviews:

  • The single-row reserve should cover the per-task and kNN lanes too: interactive calls were refused about half the time under saturation.
  • Capacity refusals shouldn't use up DRF throttle allowance. The Mapper should tell the two 429s apart: today it shows "Waiting for capacity" for a rate limit as well.
  • The Mapper's concurrency vs per-user limits, and fair retry order.
  • The per-tab gate across project switches.
  • Interactive calls waiting behind bulk pauses.
  • Retrying ambiguous failures after long calls.
  • Retrying skipped releases once Redis is back.

The full readout, reviews and evidence are on OpenConceptLab/ocl_online#275.

@paynejd
paynejd merged commit 3aae55e into master Oct 1, 2026
3 checks passed
@paynejd
paynejd deleted the ocl_online-275-capacity-limit branch October 1, 2026 14:23
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