Skip to content

feat: manage background workers in the server lifecycle#96

Merged
marnickvda merged 5 commits into
mainfrom
feat/background-workers
Jun 15, 2026
Merged

feat: manage background workers in the server lifecycle#96
marnickvda merged 5 commits into
mainfrom
feat/background-workers

Conversation

@marnickvda

@marnickvda marnickvda commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Description

Adds a Worker type and a Workers field on Config so callers can register long-running background tasks (such as a Pub/Sub listener) that share the server lifecycle. Each worker runs in its own goroutine, receives the shutdown-aware context, recovers from panics, and logs-and-continues on error. On shutdown, workers are drained within the existing graceful-shutdown budget.

Types of Changes

  • New feature (non-breaking change which adds functionality)

Review

  • Tests
  • Documentation

Add a Worker type and a Workers field on Config so callers can register
long-running background tasks (e.g. a Pub/Sub listener) that share the
server lifecycle. Workers run in their own goroutines, receive the
shutdown-aware context, recover from panics, and log-and-continue on
error. On shutdown they are drained within the graceful-shutdown budget.
@marnickvda marnickvda requested a review from a team as a code owner June 12, 2026 13:48
A worker following the documented contract returns ctx.Err() on
shutdown, which logged at ERROR level on every graceful shutdown.
Skip context.Canceled/DeadlineExceeded, mirroring the existing
http.ErrServerClosed handling in serve().

Also strengthen worker tests: table-driven TestRunWorker (incl. a
context-cancellation case), goleak.VerifyNone on the serve tests,
and deterministic testing/synctest for the drain timing tests.
Comment thread fastecho.go
Comment on lines +397 to +400
err := s.Echo.Shutdown(shutdownCtx)

// Wait for workers to unwind, bounded by the same shutdown budget.
s.drainWorkers(shutdownCtx, &workers)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is this the right order or they should be the other way around? 🤔 I legit don't know 😬

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes (most likely) is. It first shuts down the possibility of new requests to this server, and then closes the workers. If we do it the other way around, new requests (that the worker might depend on) could come in while the workers are already disabled.

@marnickvda marnickvda merged commit 84ca63c into main Jun 15, 2026
4 checks passed
@marnickvda marnickvda deleted the feat/background-workers branch June 15, 2026 10:45
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.

3 participants