fix(http_server): register disabled endpoint synchronously to avoid stuck 503 on restart - #470
Open
masnnuller wants to merge 1 commit into
Open
Conversation
…tuck 503 When an http_server input uses the service-wide HTTP server (no dedicated address), loop()'s teardown registered a disabled 503 "Endpoint disabled." handler from an asynchronous goroutine that waited on the same HasStopped signal as WaitForClose. TriggerHasStopped released both concurrently, so the 503 registration raced with the new instance's normal-handler registration on restart (streams manager Update = Delete + Create). Since RegisterEndpoint is last-writer-wins per path, the 503 handler could occasionally win, leaving the endpoint permanently returning 503. Register the disabled handler synchronously, after handlerWG.Wait() drains in-flight requests and before TriggerHasStopped(). This guarantees the new instance's registration always happens after ours and wins, so the endpoint recovers. Hard-stop no longer needs a separate goroutine because in-flight handlers already observe HardStopChan and unblock handlerWG.Wait(), and new requests still receive 503 from the existing handler's soft-stop check during draining. Applies the same fix to the wasm build. Adds a regression test exercising repeated create/use/stop cycles on a shared service-wide server. Fixes redpanda-data#469 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #469.
When an
http_serverinput runs on the service-wide HTTP server (no dedicatedaddress) and its stream is restarted/updated in stream mode (streams managerUpdate=Delete+Create), the endpoint can very rarely get stuck returning503 Service Unavailableforever and never recover.loop()'s teardown registered a disabled 503"Endpoint disabled."handler from an asynchronous goroutine that waited on the sameHasStoppedsignal asWaitForClose.TriggerHasStopped()released both concurrently, so the 503 registration raced with the new instance's normal-handler registration on restart. SinceRegisterEndpointis last-writer-wins per path, the 503 handler could occasionally win, leaving the endpoint permanently stuck at 503.Fix
Register the disabled handler synchronously, after
handlerWG.Wait()drains in-flight requests and beforeTriggerHasStopped(). This guarantees the ordering:so the new instance's registration always happens last and the endpoint recovers.
The separate goroutine is no longer needed for hard-stop: in-flight handlers already observe
HardStopChan()and unblockhandlerWG.Wait(), and new requests still receive 503 from the existing handler's soft-stop check during draining, so behavior is preserved for the full-shutdown case.Same fix applied to the
wasmbuild (input_http_server_wasm.go).output_http_serveris not affected: it does not re-register a disabled 503 handler on stop.Testing
TestHTTPServerSharedServerRestartRecovers, which repeatedly creates/uses/stops anhttp_serverinput on a shared service-wide server and asserts the endpoint recovers (200) each time. It uses the realapi.Typeregistry soRegisterEndpointoverwrite semantics match production.go build(default +GOOS=js GOARCH=wasm),go vet, and the fullinternal/impl/io/suite pass with-race.