Skip to content

feat(replacer): Tune REPLACE resources via a sentry-option - #8380

Merged
phacops merged 6 commits into
masterfrom
perf/replacer-replace-resources
Aug 22, 2026
Merged

feat(replacer): Tune REPLACE resources via a sentry-option#8380
phacops merged 6 commits into
masterfrom
perf/replacer-replace-resources

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Errors-replacer FINAL inserts can take more ClickHouse threads, block size, and memory per environment without another Snuba deploy.

A replacer sentry-option overlays those three knobs at query time. Defaults stay 1 thread / 512-row blocks / 10GiB. Missing fields fall back to REPLACER_MAX_THREADS, REPLACER_MAX_BLOCK_SIZE, and REPLACER_MAX_MEMORY_USAGE. The rest of the REPLACE client profile is unchanged.

replacer:
  max_threads: 32
  max_block_size: 65536
  max_memory_usage: 68719476736  # 64GiB

Co-Authored-By: PDPM <pierre.massat@sentry.io>
@sentry-junior
sentry-junior Bot requested a review from phacops August 22, 2026 06:14
@phacops
phacops marked this pull request as ready for review August 22, 2026 06:15
@phacops
phacops requested a review from a team as a code owner August 22, 2026 06:15

@phacops phacops left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like the requested change: REPLACE-only settings, no SQL, 32 / 65536 / 64GB.

Two things before this ships:

Prod already overrides memory. ops/k8s/services/snuba/templates/prod_settings.py.j2 sets REPLACER_MAX_MEMORY_USAGE = 50GB for every SaaS/ST environment. After this merges, SaaS stays at 50GB unless ops is updated. Threads and block size will apply in prod (those are not overridden). If the 80-core / 320GB errors boxes are the target, this PR only gets you 32 threads and 64k blocks there — still 50GB RAM.

These are now the defaults for everyone. Self-hosted / docker / CI inherit 32 threads and a 64GB max_memory_usage. That does not reserve 64GB, it only raises the per-query kill. On a laptop-sized ClickHouse it just means a runaway FINAL can OOM the host instead of aborting at 10GB. Fine if we accept that, but it is a behavior change outside SaaS.

Comment next to the constants cites “~80 CPUs / 320GB” as if that is the default deployment. It is not. I would drop the hardware numbers from the default-settings comment (keep them in the PR body) so self-hosted readers do not think this is sized for their box.

Otherwise the wiring is correct: max_threads is actually sent on the REPLACE client, tests pin the literals, and the errors user profile really is max_threads=1 unless the query overrides it.

@sentry-junior sentry-junior Bot changed the title perf(replacer): Raise REPLACE threads, block size, and memory perf(replacer): Make REPLACE resource limits sentry-options Aug 22, 2026
Keep the historical 512-row / 10GB REPLACE defaults. Raise max_threads, max_block_size, and max_memory_usage per environment through sentry-options applied at query time (0 keeps the code / CH user default).
Comment thread snuba/clusters/cluster.py
@phacops phacops changed the title perf(replacer): Make REPLACE resource limits sentry-options feat(replacer): Override REPLACE resources via sentry-options Aug 22, 2026
@phacops
phacops force-pushed the perf/replacer-replace-resources branch from 8595bee to 7226354 Compare August 22, 2026 06:23
Expose max_threads, max_block_size, and max_memory_usage as fields on a single replacer sentry-option. Defaults stay 1 / 512 / 10GiB so environments can raise only the knobs they need.
Add REPLACER_MAX_THREADS=1 next to the existing block-size and memory settings. Missing or non-positive replacer option fields use those values instead of 0.
Drop the duplicated replacer defaults dict. Each field uses the matching REPLACER_* setting when the option store is down or the field is missing or non-positive.
@phacops phacops changed the title feat(replacer): Override REPLACE resources via sentry-options feat(replacer): Tune REPLACE resources via a sentry-option Aug 22, 2026
Trust the replacer sentry-option fields as-is. Fall back to the matching REPLACER_* setting only when a field is missing.
@phacops
phacops enabled auto-merge (squash) August 22, 2026 06:36
@phacops
phacops self-requested a review August 22, 2026 06:36

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5c2c9c8. Configure here.

Comment thread snuba/replacer.py
"max_memory_usage": get_mapped_option(
"replacer", "max_memory_usage", settings.REPLACER_MAX_MEMORY_USAGE
),
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-positive values not rejected

Medium Severity

_replace_resource_settings forwards whatever get_mapped_option returns, including zero or negative values. The PR contract and the prior implementation treated non-positive fields as “use the settings default,” but that guard was removed. A 0 for max_threads or max_memory_usage can mean auto/unlimited in ClickHouse rather than the historical REPLACE profile.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5c2c9c8. Configure here.

@phacops
phacops merged commit 6100926 into master Aug 22, 2026
68 checks passed
@phacops
phacops deleted the perf/replacer-replace-resources branch August 22, 2026 06:57
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