Skip to content

Keep compatibility rescans from refusing work they confirm - #19

Merged
ancongui merged 1 commit into
mainfrom
fix/compatibility-rescan-readiness
Oct 8, 2026
Merged

ancongui merged 1 commit into
mainfrom
fix/compatibility-rescan-readiness

Conversation

@ancongui

@ancongui ancongui commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Why

Acceptance runs intermittently got 503 WV-COMPATIBILITY from the periodic compatibility rescan. There are two separate causes.

  1. Short refusal while a rescan cleans up (the CI failure on PR Add acceptance foundations: private origins, cross-platform CI and the Acme fixture #15, run 37798375200). scan() withdrew readiness in its finally before disposing the catalog engine, even when the rescan confirmed "ready". Studio's POST .../runs was admitted by the transport, logged its first run.start authorization, then hit require_operational() inside that window. /health/ready answered 200 every ~2.3 s for the whole log, and worker claims kept their 260 ms cadence, so the window lasted milliseconds. Catalog disposal takes about 2 ms against a real PostgreSQL.
  2. About a minute restricted after a busy moment (local reproduction). The rescan classified each retained row with execute_pure(control=True) and no lease of its own. Every in-flight GET holds one of the four control slots for its whole duration, and the Studio host forwards up to eight concurrent requests. When four reads were in flight, classification raised CatalogError(429, WV-OPERATION-CAPACITY). The scan recorded it as inventory_incomplete, the verdict became restricted, effects were closed, and every change answered 503 until the next rescan. In both runs GETs were also getting 429 at the same time. In CI those 429s don't line up with any database lock timeout, so the control slots really do saturate in this flow.

What changes

The owner approved these semantics before implementation.

  • Dedicated inventory slot. inventory_execution() in operations/execution.py is a one-slot lease that the scan holds as the inherited request lease for its traversal. A request burst can no longer refuse classification. If a cancelled scan's classification thread still holds the slot, the next scan is refused, which still fails closed.
  • Confirmed readiness stays in force. When the published verdict is ready and the rescan confirms it (traversal finished, complete, no incomplete or blocking findings), readiness is kept through bounded catalog cleanup and the no-op on_ready. A failed cleanup, a failed callback, or an interrupted traversal still withdraws it. Restricted→ready and ready→restricted transitions keep their existing order.
  • Diagnostics. When a rescan turns ready into restricted, the server logs Compatibility rescan withdrew readiness: findings=<kind:code,...> error=<Class [CODE]>. Exception messages and connection details are never logged.
  • Retry-After on 503 WV-COMPATIBILITY. CatalogError gains an optional retry_after. ConnectorRegistry.require_operational() fills it from CompatibilityService.retry_after_seconds(), the seconds until the next automatic rescan. Both ErrorAdvice and the BodyBoundary middleware render it.
  • Studio replays a read, or a change that carries an Idempotency-Key, after 503 WV-COMPATIBILITY within its existing limit of four attempts, but only when Retry-After is 5 seconds or less. A longer restriction fails at once.
  • Docs. reference/api.md (retries), operations/upgrades.md (rescan behavior and the warning) and operations/troubleshooting.md (a new symptom row).

Tests

  • tests/unit/operations/test_compatibility_rescan.py covers:
    • a request burst during a rescan
    • the confirmed verdict kept through cleanup, and withdrawn when cleanup fails, when the callback fails, or on an interrupted traversal
    • the transition into ready staying withdrawn
    • the withdrawal warning, and no log for restricted→restricted
    • Retry-After counting down, and refusals carrying Retry-After
  • tests/unit/operations/test_execution.py: the inventory slot is independent of request admission and covers the actual thread lifetime.
  • tests/unit/operations/test_transport.py: Retry-After on both the middleware refusal and an in-request refusal.
  • tests/integration/test_operations.py with the real app and PostgreSQL:
    • a rescan stays ready while four reads hold every control slot (a fifth read gets 429)
    • POST /runs is admitted while a confirming rescan cleans up
    • the existing restricted test now also checks Retry-After
  • On origin/main, both new integration tests fail for the reasons above: restricted [inventory:inventory_incomplete], and ready == False during cleanup.
  • studio/tests/api.test.ts: replay of a keyed change, no replay without a key, no replay for other 503s, and an immediate failure for a long Retry-After.

Verification (local)

  • ruff, ruff format, mypy, check_docs.py, source_coverage.py --strict, mkdocs build --strict: pass
  • pytest tests/unit tests/contracts: 4389 passed. After review follow-ups, tests/unit/operations and tests/unit/connectors were rerun: 422 passed.
  • Integration files touching compatibility, the operational guard, runs, composition, restart, connections, definitions and providers, against a disposable PostgreSQL: 197 passed
  • Studio: npm run check, format:check, npm test (778), npm run build, npm run test:browser (921 passed, 6 skipped)
  • Full tests/integration against a disposable PostgreSQL only: 571 passed, 1 skipped. The other 59 stop at their explicit prerequisites, which were not provided locally: an owned Kafka image, a Keycloak endpoint, the PostgreSQL connector control database, the release Docker context and a built native image.

Not run: the PR #15 acceptance harness, which is not on main yet.

Follow-up

RecoveryLoop and other lifespan loops also borrow request capacity for pure execution. PR #22 (fix/background-loop-admission) handles that with ReservedSlots, which reserves one slot per pure call. inventory_execution() stays separate: it holds one lease for the whole traversal so that an overlapping rescan is refused. PR #22 reports that the two merge cleanly.

The periodic rescan classified retained items through control-slot pure
execution without a lease of its own. When in-flight reads held all four
control slots, the capacity refusal was recorded as inventory_incomplete,
the process turned restricted and closed its effects until the next rescan.
The scan now holds a dedicated inventory slot for its traversal.

A rescan that confirmed a ready verdict still withdrew readiness before
releasing its catalog connection, so a mutation that reached
require_operational() in that window answered 503 WV-COMPATIBILITY. A
confirmed ready verdict now stays in force through bounded catalog cleanup
and the no-op ready callback. A failed cleanup, a failed callback or an
interrupted traversal still withdraws it, and restricted-to-ready and
ready-to-restricted transitions keep their order.

When a rescan withdraws readiness, the server logs the finding kinds and
codes and the class of the error it caught, never messages or connection
details. A WV-COMPATIBILITY refusal carries Retry-After with the seconds
until the next automatic rescan, and Studio replays reads and keyed changes
when that falls within its five-second pause.
@ancongui
ancongui merged commit f043688 into main Oct 8, 2026
15 checks passed
@ancongui
ancongui deleted the fix/compatibility-rescan-readiness branch October 8, 2026 17:37
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