: remove controller health monitoring #2058
Open
+271
−326
Conversation
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
Summary: decouple monarch_hyperactor from hyperactor_multiprocess. move the hyperactor_multiprocess-dependent proc `bootstrap` and `world status` helpers out of monarch_hyperactor into monarch_extension. introduce a new `monarch_extension::proc` module (exported as `monarch._rust_bindings.proc`) that provides `init_proc` and `world_status` using hyperactor_multiprocess. update rust_backend_mesh.py to import those functions from the new module. as a result monarch_hyperactor drops its hyperactor_multiprocess dependency, pickled client actors expose `instance_arc()` for the new `world_status` helper, and legacy controller supervision polling is removed from `monarch_hyperactor::proc` since it depended on the system actor and is only used by the old rust_backend_mesh path. Reviewed By: vidhyav, dulinriley Differential Revision: D88417111
Summary: the entire controller health monitoring system is now dead code. in monarch_hyperactor/src/proc.rs, the supervision check loop was already stubbed out to just log and do nothing. the struct fields `controller_id`, `last_controller_status_check`, `controller_error_sender`, and `controller_error_receiver` were initialized but never meaningfully used. the `set_controller()` method was only called from the deprecated `ClientActor::attach()` code path. the `ControllerError` enum was never actually raised since the only code path that could raise it was inside the stubbed-out supervision check. in monarch_extension/src/client.rs, the error handling for `ControllerError` was unreachable code since that error was never raised. the `set_controller()` call in `ClientActor::attach()` was part of this deprecated flow. this diff deletes all of this supervision code. the `ensure_detached_and_alive()` method is simplified to only check signals. all controller-related struct fields are removed along with the `ControllerError` enum definition. Differential Revision: D88441603
meta-cla
bot
added
the
CLA Signed
|
@shayne-fletcher has exported this pull request. If you are a Meta employee, you can view the originating Diff in D88441603. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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:
the entire controller health monitoring system is now dead code. in monarch_hyperactor/src/proc.rs, the supervision check loop was already stubbed out to just log and do nothing. the struct fields
controller_id,last_controller_status_check,controller_error_sender, andcontroller_error_receiverwere initialized but never meaningfully used. theset_controller()method was only called from the deprecatedClientActor::attach()code path. theControllerErrorenum was never actually raised since the only code path that could raise it was inside the stubbed-out supervision check.in monarch_extension/src/client.rs, the error handling for
ControllerErrorwas unreachable code since that error was never raised. theset_controller()call inClientActor::attach()was part of this deprecated flow.this diff deletes all of this supervision code. the
ensure_detached_and_alive()method is simplified to only check signals. all controller-related struct fields are removed along with theControllerErrorenum definition.Differential Revision: D88441603