Consolidate instance lifecycle subscriptions#189
Merged
Conversation
hiroTamada
approved these changes
Apr 6, 2026
hiroTamada
left a comment
Contributor
There was a problem hiding this comment.
solid refactor — clean fan-out bus with good observability. two minor comments.
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.
Summary
lib/instancesWaitForStateto filter the lifecycle bus and keep its polling fallbackWhy This Change
We were about to introduce a second internal subscription mechanism for the upcoming auto-standby work. The existing
WaitForStatepath used a per-instance state subscription that only carriedState/StateError, while the new controller needs a global stream of lifecycle changes like create, update, fork, and delete.Instead of keeping two different in-memory buses, this change consolidates on one richer lifecycle event stream and makes
WaitForStatea consumer of that shared path. That keeps the public API the same, reduces duplicated fanout logic, and gives us one place to observe subscriber backpressure.Testing
go test ./lib/instances -run 'TestWaitForState|TestLifecycleSubscribers|TestLifecycleEventMetrics'go test ./lib/builds -run 'Test'go test ./lib/instances -run '^$'go test ./lib/builds -run '^$'go test ./cmd/api/api -run '^$'Note
Medium Risk
Changes core instance lifecycle signaling by replacing per-instance state subscriptions with a shared lifecycle event bus, which could affect wait/polling behavior and internal controllers if events are missed or dropped. Adds new buffering/config/metrics paths that need validation in production workloads.
Overview
Consolidates internal instance change notifications by replacing the per-instance
StateChangesubscription mechanism with a sharedLifecycleEventbus, and updates theManagerinterface to exposeSubscribeLifecycleEvents.WaitForStatenow consumes this shared stream (filtering byinstance_idand handlingdeleteexplicitly) while retaining the 5s polling fallback, and manager mutation methods (CreateInstance,UpdateInstance,Start/Stop,Standby/Restore,Fork*,DeleteInstance) publish lifecycle events on success.Adds observability and tuning for backpressure: new metrics for subscriber count, max queue depth, and dropped events, plus a new
instances.lifecycle_event_buffer_sizeconfig setting (default256) wired through providers intoinstances.ManagerConfigand validated in config/tests.Reviewed by Cursor Bugbot for commit 134628d. Bugbot is set up for automated code reviews on this repo. Configure here.