-
Notifications
You must be signed in to change notification settings - Fork 701
refactor: remove actorstatus enum and simplify actor state handling #23625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Remove the ActorStatus enum and all related actor state tracking from the meta model and controller layers to simplify the codebase and metadata handling. - Delete `actor.rs` model file and remove the `status` field from ActorModel - Remove ActorState enum and `state` field from proto `ActorStatus` message, reserving the field number for backward compatibility - Update frontend catalog to hardcode actor state as "RUNNING" instead of reading from proto - Remove all conversions and references to PbActorState and actor status in service and controller code - Eliminate code rebuilding fragment mappings based on actor states and related tests This change reflects a design decision to treat all actors as implicitly running and to externalize or remove ephemeral state tracking from metadata, reducing complexity and maintenance overhead. Signed-off-by: Peng Chen <[email protected]>
43ab7ee to
99ec052
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the concept of actor state (Inactive/Running) from the RisingWave metadata model. The changes simplify the codebase by eliminating unused state tracking infrastructure and treating all actors as running once they are created.
Key Changes
- Removed
ActorStateenum and relatedstatusfield fromActorModel - Deleted the entire
src/meta/model/src/actor.rsfile containing theActorStatusenum - Removed unused methods from
StreamJobFragments:update_actors_state,worker_actor_states,active_actors - Removed the
rebuild_fragment_mapping_from_actorshelper function - Reserved the
statefield in protobuf definitions for backward compatibility - Updated system catalog
rw_actorsto return hardcoded "RUNNING" state
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/meta/model/src/actor.rs | Entire file deleted - contained ActorStatus enum and ActorModel with status field |
| src/meta/model/src/lib.rs | Removed module declaration for actor.rs |
| src/meta/src/model/stream.rs | Removed unused methods and updated comment to reflect removal of actor status tracking |
| src/meta/src/controller/fragment.rs | Moved ActorModel definition here (without status field) and updated all usages |
| src/meta/src/controller/utils.rs | Removed status field from PartialActorLocation and deleted rebuild_fragment_mapping_from_actors function |
| src/meta/src/rpc/ddl_controller.rs | Removed status field initialization when creating actors |
| src/meta/service/src/stream_service.rs | Removed state field from ActorState response |
| src/frontend/src/catalog/system_catalog/rw_catalog/rw_actors.rs | Hardcoded state to "RUNNING" instead of reading from metadata |
| proto/meta.proto | Reserved state field in ActorStatus and ActorState messages for backward compatibility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
src/meta/src/controller/fragment.rs
Outdated
| } | ||
|
|
||
| #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] | ||
| pub struct ActorModel { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the Model key word as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just call it "Actor"? It feels a bit too general.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed it to ActorInfo and strictly limited the scope.
- Rename struct `ActorModel` to `ActorInfo` to clarify its role as actor metadata - Remove `pub` from `ActorInfo` to limit its usage to the module scope - Make `compose_table_fragments` and `compose_fragment` private to encapsulate internals - Update all function signatures and variables to use `ActorInfo` instead of `ActorModel` - Improve API encapsulation by reducing the public surface of fragment-related components Signed-off-by: Peng Chen <[email protected]>
Remove the ActorStatus enum and all related actor state tracking from the meta model and controller layers to simplify the codebase and metadata handling.
actor.rsmodel file and remove thestatusfield from ActorModelstatefield from protoActorStatusmessage, reserving the field number for backward compatibilityThis change reflects a design decision to treat all actors as implicitly running and to externalize or remove ephemeral state tracking from metadata, reducing complexity and maintenance overhead.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
close #23604 #23605
What's changed and what's your intention?
Summary
This PR removes the
ActorStatusenum and all associated actor state management logic from the meta and frontend codebases. Actor states are no longer tracked dynamically; instead, actors are assumed to be always"RUNNING", simplifying the system’s handling of actor lifecycle and status.Details
ActorStateenum and thestatefield from theActorStatusprotobuf message, reserving the field number for backward compatibility.actor.rsmodel containingActorStatusand thestatusfield fromActorModel."RUNNING"string (notably in the frontend catalog query and stream service).rebuild_fragment_mapping_from_actors).rw_actorsto report a constant actor state, reducing complexity and potential state inconsistencies.Checklist