Repository navigation
[Rust Frontend][gRPC] Add abort control RPC #49255
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
Changes from 3 commits
4d1eeea
ebff451
be95071
ad7bb8f
0671880
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -26,8 +26,10 @@ pub mod pb { | |||||
| } | ||||||
|
|
||||||
| pub(crate) use health::monitor_health; | ||||||
| pub use pb::control_server::ControlServer; | ||||||
| pub use pb::generate_server::GenerateServer; | ||||||
|
|
||||||
| pub(crate) type ControlGrpcService = ControlServer<ControlServiceImpl>; | ||||||
| pub(crate) type GenerateGrpcService = GenerateServer<GenerateServiceImpl>; | ||||||
|
|
||||||
| #[cfg(test)] | ||||||
|
|
@@ -44,6 +46,36 @@ impl GenerateServiceImpl { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| /// gRPC control service backed by the shared application state. | ||||||
| pub struct ControlServiceImpl { | ||||||
| state: Arc<AppState>, | ||||||
| } | ||||||
|
Comment on lines
+49
to
+52
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would still be great if we could make each service a separate module/file! |
||||||
|
|
||||||
| impl ControlServiceImpl { | ||||||
| pub fn new(state: Arc<AppState>) -> Self { | ||||||
| Self { state } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| #[tonic::async_trait] | ||||||
| impl pb::control_server::Control for ControlServiceImpl { | ||||||
| async fn abort( | ||||||
| &self, | ||||||
| request: Request<pb::AbortRequest>, | ||||||
| ) -> Result<Response<pb::AbortResponse>, Status> { | ||||||
| let request_ids = request.into_inner().request_ids; | ||||||
| if request_ids.is_empty() { | ||||||
| return Ok(Response::new(pb::AbortResponse {})); | ||||||
| } | ||||||
|
Comment on lines
+67
to
+69
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a recent change on the semantics of empty list: now it cancels all in-flight requests (although there could be some race cases), so we get some misalignments here. cc @njhill WDYT of the desired behavior here? vllm/rust/src/server/src/routes/abort_requests.rs Lines 26 to 27 in c01618f
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes agree it would be better to have this be a no-op. Is the "abort all" behavior really needed, I don't think we have it in the existing abort apis. Maybe we could leave it out for now and consider as a follow-on. |
||||||
| self.state | ||||||
| .chat | ||||||
| .abort(&request_ids) | ||||||
| .await | ||||||
| .map_err(|error| Status::internal(error.to_report_string()))?; | ||||||
| Ok(Response::new(pb::AbortResponse {})) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| #[tonic::async_trait] | ||||||
| impl pb::generate_server::Generate for GenerateServiceImpl { | ||||||
| type GenerateStreamStream = | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.