Fix reload reconciliation of keyed children#72
Closed
samuel-williams-shopify wants to merge 1 commit into
Closed
Conversation
Assisted-By: devx/904563b8-dbee-48b0-9726-f036df3ed96d
8f014d2 to
af85ecc
Compare
Contributor
Author
|
Superseded by #73 — rather than fix the incomplete reconcile-remove, we're removing reload/keyed reconciliation and will revisit with a simpler (adoption-based) design. |
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.
Reloading a container is supposed to reconcile keyed children against the current configuration: reuse unchanged keys, spawn newly-appeared keys, and stop keys that have disappeared. The stop half was broken.
Bugs fixed
NoMethodErroron removal.Keyed#stop?called@value.stop, butForked::Child/Threaded::Childhave nostopmethod (onlyinterrupt!/terminate!/kill!). So dropping a keyed child on reload always raised. Never covered by a test.Respawn of removed
restart: truechildren. Even once stopped, arestart: truekeyed child's supervising fiber would immediately respawn it (therestart && !@stoppinggate is container-wide). Falcon's virtual hosting spawns exactly these (spawn(restart: true, key: path)).wait_until_readyhang after a reuse/remove-only reload.wait_until_readyslept before checking readiness, so a reload that spawned no new child (nothing sends a readiness message) blocked forever inselecteven though everything was already ready.Changes
Group#stop_child(channel, graceful)— stop a single child with the same multi-phase sequence asGroup#stop(SIGINT + wait, then SIGKILL + wait).Channel#stopping!/#stopping?— a per-child flag, checked by the supervising fiber's restart gate, so a deliberately-stopped child is not respawned. Lives on the child (whose lifetime matches the flag and which resets naturally on restart), keeping the child-reportedstatehash free of container control flags.Generic#stop_childmarks the child stopping and delegates to the group; the supervising fiber removes the entry from@keyed/@statewhen the child exits.Generic#reloadsnapshots keyed values and stops any that were not re-marked (outside the@keyediteration, to avoid re-entrant mutation).Generic#wait_until_readychecks readiness before sleeping.Keyed#stop?.Tests
Adds two behavioural tests under
with "#reload": spawns a newly configured keyed child and stops a keyed child that is no longer configured. The removal test is the regression guard. Full suite (193) green.Scope note: this implements synchronous (blocking) reap in
reload, correct for the current (non-reactor) controller. Once the controller runs under Async,Group#stop_childandChannel#stopping!can be replaced by per-childAsync::Taskcancellation.