8000 feat(ess): implement resumption stream consumer and worker by andrzejfudro · Pull Request #890 · google/xmc · GitHub
[go: up one dir, main page]

Skip to content

feat(ess): implement resumption stream consumer and worker - #890

Merged
andrzejfudro merged 4 commits into
mainfrom
feat/ess-resumption-stream-consumer
Sep 16, 2026
Merged

andrzejfudro merged 4 commits into
mainfrom
feat/ess-resumption-stream-consumer

Conversation

@andrzejfudro
@andrzejfudro andrzejfudro commented Sep 16, 2026 •
Copy link
Copy Markdown
Collaborator

This code is currently untested but is behind the feature flag. In parallel I'm working on PR to push events to this stream and I'll verify it then. It is splitted to avoid big PRs as this is mostly boilerplate.

Summary

Implements the background resumption stream consumer loop and resumption worker in the Experiment State Server (ESS) for Task 13 of the Pause Scheduling feature.

Key Changes

  • Resumption Consumer: Encapsulates Redis stream consumer logic listening to xmc-scheduling-resume-stream via consumer group ess-group, parsing trigger payload (batch_size, max_backlog_threshold), and dispatching to ResumptionWorker.
  • Lifecycle & Reliability: Implements a 5-second bounded drain timeout in Close() to prevent Kubernetes SIGKILLs and manages worker lifecycles with an internal sync.WaitGroup to avoid server shutdown deadlocks.
  • Server Composition: Plugs the resumption consumer into ProdServer behind the PauseSchedulingEnabled feature flag and registers teardown in the LIFO cleanup stack while keeping server/BUILD.bazel free of Redis client dependencies.
  • Tests: Table-driven unit tests for payload parsing, handler DLQ routing, resumption worker, and consumer startup/drain timeout.

Verification

  • bazel test //cmd/experiment_state_server/resumption/... (Passed)
  • bazel test //cmd/experiment_state_server/server:server_test (Passed)
  • bazel test //cmd/experiment_state_server:health_test (Passed)
  • bazel build //cmd/experiment_state_server:experiment_state_server (Passed)

TAG=agy
CONV=b35410bc-068d-4e00-bae0-22b3ca180c39

Implement background consumer loop in Experiment State Server (ESS)
listening to `xmc-scheduling-resume-stream` via consumer group `ess-group`.
The consumer parses `batch_size` and `max_backlog_threshold` from the trigger
payload, dispatches execution to a stub resumption worker, and is activated
strictly behind the `PauseSchedulingEnabled` feature flag.

Encapsulate stream consumer lifecycle in `resumption.Consumer` with bounded
graceful drain timeout and deadlock-free teardown hooks registered in the
server cleanup stack.

TAG=agy
CONV=b35410bc-068d-4e00-bae0-22b3ca180c39
@gemini-code-assist gemini-code-assist Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new resumption package to coordinate background consumption of the scheduling resumption stream from Redis when the PauseSchedulingEnabled feature flag is active. It includes a consumer, a task handler, payload parsing with defaults, a stub worker, and comprehensive unit tests. The review feedback highlights two key improvements: adding defensive nil checks for the handler and its worker in HandleTask to prevent potential runtime panics, and fixing a goroutine leak in TestConsumer_DrainTimeout by ensuring the simulated in-flight worker's WaitGroup is properly decremented.

8000
Add defensive nil checks for handler and worker in HandleTask to prevent
runtime panics if invoked on a nil receiver or uninitialized worker.
Add defer consumer.wg.Done() in TestConsumer_DrainTimeout to ensure the
background wait goroutine unblocks and exits cleanly after test completion.

TAG=agy
CONV=b35410bc-068d-4e00-bae0-22b3ca180c39
Rename StubWorker to ResumptionWorker and NewStubWorker to
NewResumptionWorker to establish the final domain worker type
ahead of the downstream Spanner replay worker implementation (Task 15a).

TAG=agy
CONV=b35410bc-068d-4e00-bae0-22b3ca180c39
@andrzejfudro andrzejfudro changed the title feat(ess): implement resumption stream consumer and stub worker feat(ess): implement resumption stream consumer and worker Sep 16, 2026
@andrzejfudro
andrzejfudro marked this pull request as ready for review September 16, 2026 10:34
janzak-dev
janzak-dev previously approved these changes Sep 16, 2026
… logging

- Call c.wg.Add(1) adjacent to each worker goroutine instead of c.wg.Add(2).
- Remove redundant WIP log when PauseSchedulingEnabled is false in
  cmd/experiment_state_server/server/prod.go.

TAG=agy
CONV=b35410bc-068d-4e00-bae0-22b3ca180c39
@andrzejfudro
andrzejfudro added this pull request to the merge queue Sep 16, 2026
Merged via the queue into main with commit dbcdd87 Sep 16, 2026
11 checks passed
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.

2 participants

0