feat(replacer): Tune REPLACE resources via a sentry-option - #8380
Conversation
Co-Authored-By: PDPM <pierre.massat@sentry.io>
phacops
left a comment
There was a problem hiding this comment.
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.
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).
8595bee to
7226354
Compare
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.
Trust the replacer sentry-option fields as-is. Fall back to the matching REPLACER_* setting only when a field is missing.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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.
| "max_memory_usage": get_mapped_option( | ||
| "replacer", "max_memory_usage", settings.REPLACER_MAX_MEMORY_USAGE | ||
| ), | ||
| } |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 5c2c9c8. Configure here.


Errors-replacer FINAL inserts can take more ClickHouse threads, block size, and memory per environment without another Snuba deploy.
A
replacersentry-option overlays those three knobs at query time. Defaults stay 1 thread / 512-row blocks / 10GiB. Missing fields fall back toREPLACER_MAX_THREADS,REPLACER_MAX_BLOCK_SIZE, andREPLACER_MAX_MEMORY_USAGE. The rest of the REPLACE client profile is unchanged.