Skip to content

Avoid registry lock during first-byte attribution - #5

Merged
numinousmuses merged 1 commit into
masterfrom
forge/task-30589095b35cf00ef20dee1214ba0e27
Sep 6, 2026
Merged

numinousmuses merged 1 commit into
masterfrom
forge/task-30589095b35cf00ef20dee1214ba0e27

Conversation

@numinousmuses

@numinousmuses numinousmuses commented Sep 6, 2026

Copy link
Copy Markdown

A registry write lock could stall the first response bytes after the provider had already received the request. Serving-slot bookkeeping looked the provider up again through Registry.GetProvider during primary dispatch, backup re-latching, and live winner attribution.

Use the matching provider already held by the dispatch state. Preserve frozen terminal attribution and existing paged/contiguous expectations; an absent or mismatched provider yields unknown attribution. The deterministic HTTP regression holds the registry lock before the latch executes, while helper tests cover backup and winner paths.

flowchart LR
  subgraph Before
    B1[Provider receives request] --> B2[dispatchPrimary or backup race]
    B2 --> B3[noteServingSlotFor or kvBackendAttribution]
    B3 --> B4[Registry.GetProvider read lock]
    B4 --> B5[Wait behind registry writer]
    B5 --> B6[First client bytes delayed]
  end
Loading
flowchart LR
  subgraph After
    A1[Provider receives request] --> A2[dispatchPrimary or backup race]
    A2 --> A3[noteServingSlotFor or liveSlotBackend]
    A3 --> A4[Matching retained Provider.SlotKVBackendTags]
    A4 --> A5[First client bytes without global registry lookup]
    A3 --> A6[Missing or mismatched provider yields unknown]
    A2 --> A7[Terminal snapshot stays frozen]
  end
Loading

Validation: focused race tests pass three repetitions. Restoring the old lookups makes the deterministic HTTP regression fail at its original two-second assertion and makes the affected helper tests fail. The original first-byte assertion remains unchanged. Forge separately verifies the complete coordinator race suite with real PostgreSQL and checks documentation, then PR CI verifies the merge with the target branch.

The task history retains the first incomplete candidate, a superseded source revision, and a timed-out implementation. Its successful edits were recovered from the retained transcript and reviewed before this continuation. The timeout itself was not treated as a verified result. Publication evidence below identifies the final candidate and separate verification runs.

This change was prepared by Numinous Forge and independently verified on a fresh worker.

Task: 30589095b35cf00ef20dee1214ba0e27
Base: 9ad2aa17bcb13598804d074c3cdada2aeed80b72
Verified tree: 73959842c03bd84c15a1fd52e99695888909e72a
Policy: f6b77f17eafcfc1feefc4567319da0e564d02ed90cc71129956ec97444f4781d
Verification runs: 5acedec2fdea5c235c246cedc079b22f, 7691910dcb0e9db936f1356ca80fb7e2

Export full evidence with numinous-forge task export 30589095b35cf00ef20dee1214ba0e27 ./evidence.

Before:

flowchart LR
  Source[Original source] --> Check[Regression or acceptance check]
  Check --> Finding[Task finding]
Loading

After:

flowchart LR
  Change[Candidate source] --> Verify[Independent accepted checks]
  Verify --> Review[Maintainer review]
Loading

This draft requires maintainer review of behavior, tests, and any workflow changes before merge.

Verified tree: 7395984
Policy: f6b77f17eafcfc1feefc4567319da0e564d02ed90cc71129956ec97444f4781d
@numinousmuses
numinousmuses marked this pull request as ready for review September 6, 2026 01:53
@numinousmuses
numinousmuses merged commit bb28702 into master Sep 6, 2026
2 checks passed
@numinousmuses

Copy link
Copy Markdown
Author

Numinous Forge · Change and verification

Result: repaired and merged into this fork. This is a separate streaming problem found while checking the reconnect work.

What was wrong: A provider could already be processing a request while the client waited for its first response bytes. Response bookkeeping needed another registry lookup, which could wait behind the global registry write lock.

What changed: Use the matching provider already held by the request. Primary dispatch, backup dispatch, and live response attribution no longer need that extra global lookup. Finished-request attribution stays frozen; missing or mismatched providers yield unknown attribution.

What we proved:

  • A real HTTP regression deliberately holds the registry lock and checks that the first response bytes still arrive.
  • Restoring the old lookups makes the regression fail. The original two-second deadline was kept.
  • Focused race tests passed three repetitions. Independent coordinator checks with real PostgreSQL and documentation checks passed, followed by protocol, coordinator, and documentation checks on the PR merge.

Review focus: Provider identity must match the request, including backup paths. Finished-request attribution must not change when a later serving attempt begins.

Earlier incomplete and timed-out attempts were retained and followed by a verified revision; their output was not accepted as a passing result.

Read the code changes

Review summary for 9fadac3bf465. Live task and CI progress are reported separately.

@numinousmuses numinousmuses added the trigger:instructed Work was explicitly requested; generated PRs inherit this trigger. label Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

trigger:instructed Work was explicitly requested; generated PRs inherit this trigger.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant