Skip to content

feat(storage): opt-in lossless watch delivery + stop watch workers on shutdown - #149

Merged
benitogf merged 1 commit into
mainfrom
chore/lossless-watch-leak-fix
Jun 16, 2026
Merged

feat(storage): opt-in lossless watch delivery + stop watch workers on shutdown#149
benitogf merged 1 commit into
mainfrom
chore/lossless-watch-leak-fix

Conversation

@benitogf

Copy link
Copy Markdown
Owner

Summary

Adds an opt-in lossless mode for real-time event delivery, and stops background watch workers from lingering after their owner shuts down. Closes #148.

What changed

  • Opt-in lossless delivery. When enabled, a real-time event waits for a stalled-but-live consumer instead of being dropped after the send timeout. The default is unchanged — production still drops a stuck/dead consumer's event so a slow subscriber can never permanently hang writers.
  • Watch workers stop on owner shutdown. The storage watch helper now takes an owner lifetime signal, so its background workers exit when the owner is cancelled, not only when the store itself closes. A missing signal is treated as never-cancel (safe).

Behavior & compatibility

  • Default behavior is unchanged; lossless mode is strictly opt-in (off by default).
  • Breaking (Go API): the exported storage watch helper now takes a context argument. In-repo callers are updated; external callers must pass one — nil is accepted and means never-cancel.
  • Tradeoff to know: with lossless mode on, a consumer that stalls forever will block the storage instance (close included). It is intended only for callers whose consumer cannot stall indefinitely (e.g. test harnesses, tightly-coupled internal consumers).

Tests

  • Lossless mode provably does not drop where the default would — verified by mutation (removing the lossless branch makes the test fail), plus a contrast test showing the default drops into a full buffer.
  • Watch workers exit on shutdown signal (no goroutine leak); nil-signal is handled safely.
  • Full suite green under -race.

🤖 Generated with Claude Code

…lback

Add Options.LosslessWatch (and Server.LosslessWatch) to make storage->broadcast
event sends block instead of dropping after SEND_TIMEOUT when a consumer stalls.
Default unchanged (drop, for production write-hang resilience); enable where a
dropped broadcast would desync a subscriber, notably deterministic tests.

Make WatchWithCallback take a context so its per-shard goroutines exit on
cancel, not only on storage Close — fixes a goroutine leak when a caller
attaches a storage it does not own and detaches via context cancellation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@CBosch101 CBosch101 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.

Opt-in lossless watch delivery plus a context-cancellable WatchWithCallback to stop per-shard watchers when an owner detaches without closing the store. Clean, well-tested, default behavior preserved. The breaking signature change is already documented in the PR body and softened by accepting nil. APPROVE.

Verified

  • Default path is unchanged: blocking defaults false and sendTimeout defaults to SEND_TIMEOUT (storage.go:105,159-163), so non-opt-in callers still hit SendWithTimeout(event, SEND_TIMEOUT) exactly as before.
  • Blocking send can't race Close: the raw send runs under l.mutex.RLock in sendEvent (layered.go:828-832) while Close takes the write lock and nils the watcher (layered.go:173-179) — mutually exclusive, no send-on-closed-channel.
  • SetBlocking runs under Start's write lock and is an atomic.Bool (layered.go:114, storage.go:88-89) — no race with concurrent sendEvent; -race is green for the watch tests.
  • The blocking test is mutation-grounded (deleting the s.blocking.Load() branch flips the negative assertion), and the drop-mode contrast test proves the buffer genuinely overflows.
  • nil ctx is treated as never-cancel rather than panicking on ctx.Done() (layered.go:845-847).

@benitogf
benitogf merged commit cb88685 into main Jun 16, 2026
6 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.

Subscribers can miss real-time updates under heavy load

2 participants