Skip to content

refactor(api): remove Drain RPC, consolidate proto files, trim EngineError - #28

Merged
connorcarpenter15 merged 2 commits into
mainfrom
refactor/remove-drain-rpc
Jul 23, 2026
Merged

refactor(api): remove Drain RPC, consolidate proto files, trim EngineError#28
connorcarpenter15 merged 2 commits into
mainfrom
refactor/remove-drain-rpc

Conversation

@connorcarpenter15

@connorcarpenter15 connorcarpenter15 commented Jul 23, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #27. Two related contract cleanups in one PR:

  1. Remove the Control.Drain RPC (+ DrainRequest/DrainResponse/DrainState).
  2. Consolidate the proto file set 10 → 8 and trim dead message/field surface.

Package name, message names, and field numbers are unchanged, so the consolidation is wire-compatible and transparent to whole-package codegen — only which file a message lives in changes.

1. Drain removal

Graceful shutdown doesn't need an engine-facing drain RPC. The Dynamo sidecar (the single OpenEngine client) can drain without it: stop admitting (discovery unregister), let in-flight Generate streams complete, poll GetLoad for quiescence, and cancel stragglers via gRPC stream cancellation into the engine's existing Abort path.

Dropped lifecycle.proto's now-unused error.proto import. Kept the drain-adjacent state signals that stand on their own — HEALTH_STATE_DRAINING, KvConnectorInfo.supports_drain (validated by the sidecar for disagg connectors), and ERROR_CODE_DRAINING — since an engine still drains on its own SIGTERM.

2. Consolidation

  • observability.protoserver.proto. After the runtime-events removal it held only load reporting (GetLoadRequest/LoadInfo/RankLoadInfo) — "observability" had become a misnomer. Load is the dynamic half of what server.proto already reports (static identity/capacity/roles).
  • input.protogeneration.proto, its sole remaining consumer. TokenIds/Modality/MediaItem move as-is.

3. Dead-code / field trim

  • Task-input types (TaskInput, MultimodalTaskInput, TaskInputType) — orphaned when Embed/Classify/Score were removed. Deleted.
  • EngineError fields — dropped details (never set or read) and retry_after_ms (only ever absent), plus the now-unused struct import. Kept retryable: all three servers populate it (SGLang computes it from a 503) even though the sidecar doesn't yet read it — removing it would discard a real signal, not dead code.

Result

Control surface: GetServerInfo · GetModelInfo · GetLoad · Health · Abort · LoadLora · UnloadLora · ListLoras · GetKvEventSources · SubscribeKvEvents (+ Inference.Generate).

File set 10 → 8: openengine · generation (+input) · kv · model · server (+observability/load) · lifecycle · lora · error.

Validation

  • protoc -I proto --experimental_allow_proto3_optional builds the full descriptor set clean.
  • markdownlint-cli2 clean on all Markdown.

Consumer follow-up (separate repos, not this PR)

  • Drain: the sidecar's drain() override and the three servers' Drain handlers are removed; LLMEngine::drain reverts to the default no-op and shutdown relies on the is_quiescent/GetLoad + grace-period + stream-cancellation path already in Worker.
  • EngineError: servers stop setting details / retry_after_ms (no reader exists, so nothing breaks); retryable handling is unchanged.

Compatibility

Wire-breaking for the removed RPC and the two EngineError fields (which no consumer reads). The file consolidation itself changes no wire format.

Remove Control.Drain and its DrainRequest/DrainResponse/DrainState
messages. Graceful shutdown does not need an engine-facing drain RPC:
the client stops admitting (discovery unregister), lets in-flight
Generate streams complete, polls GetLoad for quiescence, and cancels
stragglers via gRPC stream cancellation into the engine's existing
Abort path.

Drop lifecycle.proto's now-unused error.proto import (only DrainResponse
used EngineError; Health/Abort do not). Keep the drain-adjacent state
signals that stand on their own -- HEALTH_STATE_DRAINING, supports_drain
on KvConnectorInfo (validated by the sidecar for disagg connectors), and
ERROR_CODE_DRAINING -- since an engine still drains on its own SIGTERM.

Sync docs/api.md and the README tables.

Validated with protoc and markdownlint-cli2.

Signed-off-by: Connor Carpenter <connorc@nvidia.com>
Reduce the openengine/v1 file set from 10 to 8 by folding two small
files into the domain file that owns their concern, and remove dead
message/field surface. Package, message names, and field numbers are
unchanged -- wire- and (whole-package) codegen-compatible.

- Fold observability.proto into server.proto. Post-cleanup it held only
  load reporting (GetLoadRequest/LoadInfo/RankLoadInfo); "observability"
  was a misnomer. server.proto now covers static identity/capacity plus
  dynamic load -- everything the server reports about itself.

- Fold input.proto into generation.proto, its sole remaining consumer,
  and delete the dead task-input types (TaskInput, MultimodalTaskInput,
  TaskInputType) orphaned when the non-generative task RPCs were removed.
  TokenIds/Modality/MediaItem move as-is.

- Trim EngineError: drop `details` (never set or read by any
  engine/sidecar) and `retry_after_ms` (only ever absent), and the
  now-unused struct import. Keep `retryable` -- servers populate it
  (SGLang computes it from a 503) even though the sidecar does not yet
  read it.

Sync docs/api.md and the proto README table.

Validated with protoc and markdownlint-cli2.

Signed-off-by: Connor Carpenter <connorc@nvidia.com>
@connorcarpenter15 connorcarpenter15 changed the title refactor(api): remove the Drain RPC refactor(api): remove Drain RPC, consolidate proto files, trim EngineError Jul 23, 2026
@connorcarpenter15
connorcarpenter15 marked this pull request as ready for review July 23, 2026 15:03
@connorcarpenter15
connorcarpenter15 merged commit dbd84b6 into main Jul 23, 2026
5 checks passed
@connorcarpenter15
connorcarpenter15 deleted the refactor/remove-drain-rpc branch July 23, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant