Skip to content

settings: surface validation bounds for numeric cluster settings#172040

Open
lohitkolluri wants to merge 2 commits into
cockroachdb:masterfrom
lohitkolluri:fix/172035-expose-setting-bounds
Open

settings: surface validation bounds for numeric cluster settings#172040
lohitkolluri wants to merge 2 commits into
cockroachdb:masterfrom
lohitkolluri:fix/172035-expose-setting-bounds

Conversation

@lohitkolluri

@lohitkolluri lohitkolluri commented Jun 26, 2026

Copy link
Copy Markdown

Summary

Cluster settings now expose their validation bounds. The valid range for settings like kv.snapshot_rebalance.max_rate was hidden inside the validator, so users had to discover it by trial and error or by reading error messages. This change records the min and max on each setting and shows them in the generated settings list.

Informs #172035.

What changed

Two commits.

  1. Record bounds on each setting. The four numeric setting types (int, byte size, float, duration) now store their min and max values, exposed via a new Bounds() method.

  2. Show bounds in the settings list. cockroach gen settings-list (and the generated docs it produces) now includes Min and Max columns showing the valid range for each numeric setting.

Out of scope

  • The upper-exclusive range option only reports the inclusive range today.
  • Surfacing bounds in SHOW CLUSTER SETTING, the internal settings table, and the admin HTTP API. The data is now available; making it visible there is a follow-up.

Testing

  • Added a test file covering every numeric validation option.
  • The settings package tests pass.
  • Formatter is clean.
  • Spot-checked the generated docs against representative settings (e.g. sql.trace.txn.sample_rate shows Min=0, Max=1; kv.snapshot_rebalance.max_rate shows Min=1048576, no max).

AI disclosure: Drafted with assistance from Claude Code.

Epic: CRDB-37791

@blathers-crl

blathers-crl Bot commented Jun 26, 2026

Copy link
Copy Markdown

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • When CI has completed, please ensure no errors have appeared.

I have added a few people who may be able to assist in reviewing:

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl Bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Jun 26, 2026
@blathers-crl blathers-crl Bot requested a review from rytaft June 26, 2026 21:41
Cluster-setting validation bounds (min/max) were captured only inside
opaque validateFn closures, so callers of gen settings-list, SHOW
CLUSTER SETTING, crdb_internal.cluster_settings, and the admin HTTP
API had no way to discover them without trial-and-error. For example,
sql.status.active_query_text.max_bytes accepts values in [1000 B,
64 KiB] but this is not surfaced anywhere.

This change records the bounds on the concrete numeric setting types
(IntSetting, ByteSizeSetting, FloatSetting, DurationSetting) so they
can be surfaced for documentation and introspection. A new optional
'boundsOpt' field on SettingOption is invoked after the setting is
registered, populated by the existing validation constructors
(IntInRange, IntWithMinimum, IntInRangeOrZeroDisable, ByteSize*,
DurationInRange, DurationWithMinimum, DurationWithMinimumOrZeroDisable,
FloatInRange, FloatInRangeUpperExclusive, FloatWithMinimum,
FloatWithMinimumOrZeroDisable, and the derived Fraction /
NonNegative* options). Bounds are exposed via a typed Bounds() method
on each numeric setting; nil pointer means the corresponding bound is
unbounded. The upper-exclusive semantics of FloatInRangeUpperExclusive
is intentionally not surfaced (inclusive range only) and is left for a
follow-up.

Validation behavior, error messages, and existing call sites are
unchanged. No interface changes; the change is purely additive.

Release note (general change): Expose validation bounds (min/max) of
numeric cluster settings via a new Bounds() method on
IntSetting/ByteSizeSetting/FloatSetting/DurationSetting so callers can
surface them in docs, SHOW CLUSTER SETTING, and HTTP APIs.

Release-Note: general change
Signed-off-by: Lohit Kolluri <lohitkolluri@gmail.com>
The validation bounds recorded by the new Bounds() accessors in
pkg/settings are now visible to users. Two new columns, Min and Max,
are appended to the table emitted by `cockroach gen settings-list`
and the corresponding docs/generated/settings/settings.html
genrule is regenerated. Each numeric setting now displays its bounds
in the canonical form a user would supply to SET CLUSTER SETTING
(e.g. 10, 0.5, 1h0m0s); non-numeric settings and unbounded sides
render as empty cells. `gen settings-list --without-system-only`
(settings-for-tenants.txt) is also regenerated.

Release note (general change): `cockroach gen settings-list` (and
the regenerated docs/generated/settings/settings.html and
settings-for-tenants.txt) now include Min and Max columns showing
the validation bounds of numeric cluster settings.

Release-Note: general change
Signed-off-by: Lohit Kolluri <lohitkolluri@gmail.com>
@lohitkolluri lohitkolluri force-pushed the fix/172035-expose-setting-bounds branch from b5c0e58 to 1c3acc2 Compare June 26, 2026 22:10
@blathers-crl

blathers-crl Bot commented Jun 26, 2026

Copy link
Copy Markdown

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@lohitkolluri

Copy link
Copy Markdown
Author

Heads-up: this PR covers items 1 and 2 from the issue's expected behavior:

  • Adding the Bounds() accessor for numeric settings.
  • Surfacing those bounds in the docs generator.

Item 3 (SHOW ALL CLUSTER SETTING(S), crdb_internal.cluster_settings, and /_admin/v1/settings) is intentionally out of scope for this PR. The necessary plumbing is already in place, so wiring those up should be a straightforward follow-up.

I'm happy to open a separate PR for that if you'd prefer, or leave it for whoever picks it up next.

@lohitkolluri lohitkolluri marked this pull request as ready for review June 30, 2026 19:28
@rytaft rytaft requested review from a team June 30, 2026 20:06
@rytaft

rytaft commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the contribution @lohitkolluri! I've requested a review from the owning team for cluster settings infra.

Item 3 (SHOW ALL CLUSTER SETTING(S), crdb_internal.cluster_settings, and /_admin/v1/settings) is intentionally out of scope for this PR. The necessary plumbing is already in place, so wiring those up should be a straightforward follow-up.

I'm happy to open a separate PR for that if you'd prefer, or leave it for whoever picks it up next.

Separate PR sounds good, but in that case please change "Fixes #172035" to say "Informs" so the issue doesn't get auto-closed.

@rytaft rytaft removed their request for review June 30, 2026 20:09
@lohitkolluri

Copy link
Copy Markdown
Author

Thanks! I've updated the PR to use "Informs #172035" instead of "Fixes #172035". I'll handle item 3 in a separate follow-up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-community Originated from the community X-blathers-triaged blathers was able to find an owner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants