Skip to content

Add memory limit for buffered repl changes#826

Merged
morgo merged 5 commits into
block:mainfrom
morgo:memory-limit
May 7, 2026
Merged

Add memory limit for buffered repl changes#826
morgo merged 5 commits into
block:mainfrom
morgo:memory-limit

Conversation

@morgo
Copy link
Copy Markdown
Collaborator

@morgo morgo commented May 7, 2026

A Pull Request should be associated with an Issue.

We wish to have discussions in Issues. A single issue may be targeted by multiple PRs.
If you're offering a new feature or fixing anything, we'd like to know beforehand in Issues,
and potentially we'll be able to point development in a particular direction.
Further notes in https://github.com/block/spirit/blob/main/.github/CONTRIBUTING.md

Fixes #822

@morgo morgo requested a review from Copilot May 7, 2026 17:13
@morgo morgo changed the title Memory limit Add memory limit for buffered repl changes May 7, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a per-subscription “soft” memory cap to buffered replication subscriptions to prevent wide-row workloads from growing the in-memory buffer without bound, and updates client locking to avoid deadlocks when backpressure causes HasChanged to block.

Changes:

  • Track approximate buffered bytes per subscription and apply backpressure by parking HasChanged on a condition variable once the soft limit is reached.
  • Make the per-subscription soft limit configurable via ClientConfig.SubscriptionSoftLimitBytes (default 256 MiB; negative disables).
  • Adjust client lock usage around subscription dispatch (and DDL matching) to avoid deadlocks when HasChanged parks.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/repl/subscription_buffered.go Adds byte accounting, soft-limit backpressure via sync.Cond, and logs/counters for parking.
pkg/repl/subscription_buffered_test.go Initializes cond in existing tests and adds new unit/integration tests for accounting + park/wake behavior and deadlock regression.
pkg/repl/README.md Documents the new memory backpressure behavior and operational implications.
pkg/repl/client.go Introduces default/configured soft limit, wires cond during subscription construction, and releases c.Lock before calling HasChanged.
pkg/repl/client_test.go Updates tests to initialize cond for manually constructed subscriptions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/repl/subscription_buffered.go
Comment thread pkg/repl/subscription_buffered.go
Comment thread pkg/repl/README.md Outdated
@morgo morgo enabled auto-merge May 7, 2026 18:11
@morgo morgo merged commit 208733e into block:main May 7, 2026
11 of 13 checks passed
@morgo morgo deleted the memory-limit branch May 7, 2026 18:29
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.

Buffered subscription can lead to OOM situations

3 participants