Skip to content

fix: hand off PHP requests during configuration reloads - #2648

Open
henderkes wants to merge 7 commits into
mainfrom
fix/worker-reload-queue
Open

henderkes wants to merge 7 commits into
mainfrom
fix/worker-reload-queue

Conversation

@henderkes

@henderkes henderkes commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

edit: just a reminder to myself to do it properly after api con

edit: prevent 503s from requests coming in during config reloads (complementary to nicolas' PR #2661)

@henderkes
henderkes marked this pull request as draft September 16, 2026 07:07
@henderkes
henderkes marked this pull request as ready for review September 28, 2026 21:13
@henderkes
henderkes requested review from AlliBalliBaba and dunglas and removed request for dunglas September 28, 2026 21:22
@dunglas

dunglas commented Sep 29, 2026

Copy link
Copy Markdown
Member

Could you run one of the benchmark to check the impact of the atomic.Pointer on the hot path?

@henderkes

Copy link
Copy Markdown
Contributor Author

The atomic pointer should be completely free on aarch64 and x86-64. If anything I should benchmark the check though.

@henderkes

Copy link
Copy Markdown
Contributor Author

+3% (Go) instructions per request. Not terrible, but also not perfect...

Requests that reach a PHP handler of a superseded runtime are handed to
the matching module of the new app and dispatched on the new runtime.
Hand-offs happen only before execution starts, so the body is unread,
no response was written, and the request can be dispatched as is.
@henderkes
henderkes force-pushed the fix/worker-reload-queue branch from f965bcf to b7cccf2 Compare September 29, 2026 14:17
@henderkes

Copy link
Copy Markdown
Contributor Author

instead of old requests being rerouted through new route tree, we now take their existing state and execute it on the new runtime after the reload comes back. should be +/-0 instructions

Comment thread worker.go Outdated
return nil
case workerScaleChan <- fc:
// the request has triggered scaling, continue to wait for a thread
case <-worker.done:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For a 0-runtime overhead solution you could also just have a goroutine drain all worker.requestChans after reload for some amount of time.

Comment thread worker.go
maxThreads int
requestOptions []RequestOption
requestChan chan *frankenPHPContext
done <-chan struct{}

@AlliBalliBaba AlliBalliBaba Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It probably would make sense to store and check readiness somewhere on the Server directly and not re-use them across reloads.

Would be a bit of a bigger refactor,.but IMO the behavior should be the same for regular threads and workers.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants