Skip to content

feat(consumer): per-storage max_insert_block_size override#7939

Merged
onewland merged 2 commits into
masterfrom
feat/clickhouse-max-insert-block-size
May 15, 2026
Merged

feat(consumer): per-storage max_insert_block_size override#7939
onewland merged 2 commits into
masterfrom
feat/clickhouse-max-insert-block-size

Conversation

@onewland
Copy link
Copy Markdown
Contributor

@onewland onewland commented May 15, 2026

EAP-517

Adds an optional per-storage knob for the rust consumer's ClickHouse INSERTs: when clickhouse_max_insert_block_size:<storage_name> is set to a positive integer in snuba.state, the consumer appends &max_insert_block_size=<n> to the INSERT URL. Absent or non-positive values leave the setting at ClickHouse's default.

Motivation: lets us tune the ClickHouse max_insert_block_size server setting for a single storage at runtime — for example, to shrink it on a storage hitting memory pressure during inserts — without a deploy or restart.

The key shape (<setting>:<storage_name>) and the snuba.state backing intentionally mirror the existing per-storage clickhouse_load_balancing:<storage_name> / clickhouse_load_balancing_first_offset:<storage_name> keys that already drive the same build_url path.

Considered using sentry-options for the typed schema, but it doesn't support open per-storage maps: the namespace validator force-injects additionalProperties: false at both the top level and inside object-typed options, and it doesn't permit additionalProperties: { type: integer }, so every storage name would have to be enumerated in the schema (a code change per new storage). snuba.state is the same lookup mechanism the sibling load-balancing keys already use and supports dynamic keys natively. The tradeoff is no JSON-Schema typing — values are parsed as u64 in get_max_insert_block_size and non-numeric / non-positive values are silently ignored.

Notes for review:

  • Cache TTL is the existing 10s runtime_config cache, so changes propagate within 10s of a snuba.state.set_config call.
  • A value of 0 deliberately means "leave unset" rather than "set to 0" (ClickHouse rejects 0 for this setting anyway).

Add an optional per-storage knob, read from snuba.state at
`clickhouse_max_insert_block_size:<storage_name>`, that appends
`&max_insert_block_size=<n>` to the rust consumer's INSERT URL.
Absent or non-positive values leave the setting at ClickHouse's
default.

Lets us tune insert block size on a single storage at runtime
without a deploy or restart, matching the per-storage pattern
already used for `clickhouse_load_balancing:<storage_name>`.

Co-Authored-By: Claude <noreply@anthropic.com>
@onewland onewland marked this pull request as ready for review May 15, 2026 22:44
@onewland onewland requested a review from a team as a code owner May 15, 2026 22:44
Comment thread rust_snuba/src/runtime_config.rs Outdated
Treat the per-storage `clickhouse_max_insert_block_size:<storage_name>`
override as a floor only: values below ClickHouse's compiled-in default
(1_048_449) are silently dropped instead of being applied. Pulling block
size below the server default would not raise it past what ClickHouse
already produces on its own, so the URL suffix is now only appended for
values that actually take effect.

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

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 223898c. Configure here.

Comment thread rust_snuba/src/runtime_config.rs
Comment thread rust_snuba/src/runtime_config.rs
@onewland onewland merged commit 3bd6471 into master May 15, 2026
51 checks passed
@onewland onewland deleted the feat/clickhouse-max-insert-block-size branch May 15, 2026 23:30
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.

2 participants