Remove incomplete reload/keyed reconciliation#73
Open
samuel-williams-shopify wants to merge 1 commit into
Open
Remove incomplete reload/keyed reconciliation#73samuel-williams-shopify wants to merge 1 commit into
samuel-williams-shopify wants to merge 1 commit into
Conversation
Assisted-By: devx/904563b8-dbee-48b0-9726-f036df3ed96d
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.
Reload's in-place, key-based reconciliation was incomplete and not correctly wired, so rather than ship a partial fix we're removing it and will revisit with a simpler design (likely blue-green "adoption": build the new container reusing unchanged processes, stop the rest).
Why
Controller#reloadwas orphaned — nothing operational called it (not signal-wired anywhere;SIGHUP → restart), only the tests.Keyed#stop?called@value.stop, which children don't implement →NoMethodError) and would also have respawnedrestart: truechildren.Virtual#setupuses the keyed API but never actually exercises reconciliation (itsSIGHUP → restartrebuilds a fresh container each time), so nothing relies on the working behavior.Changes
Controller#reload.Async::Container::Keyedand the container-level reconcile (mark/clear/sweep, stopping obsolete keyed children).Generic#reloadnow simply re-runs the given block; existing keyed children are still reused viaspawn(key:), so re-running setup does not duplicate them.spawn(key:)registration andcontainer[key]lookup ([]now returns the child directly).Controller#reloadtests.Not broken
Falcon only needs
spawn(key:),container[key], andcontainer.reload { ... }(which still yields) — all retained. Full suite (191) green.Supersedes #72 (which fixed the reconcile-remove bug); closing that in favour of this removal.