Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set public = t for sql.telemetry.query_sampling.max_event_frequency cluster setting #108385

Closed
florence-crl opened this issue Aug 8, 2023 · 1 comment · Fixed by #111594
Closed
Labels
A-cluster-observability Related to cluster observability C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. good first issue

Comments

@florence-crl
Copy link

This setting needs to be documented according to DOC-8029:

sql.telemetry.query_sampling.max_event_frequency

However it is not publicly documented on the cluster settings page because it has public = f in crdb_internal.cluster_settings. The cluster settings page is automatically generated with settings where public = t.

Describe the solution you'd like
Please set public = t for the above cluster setting.

@florence-crl florence-crl added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-cluster-observability A-cluster-observability Related to cluster observability labels Aug 8, 2023
@blathers-crl blathers-crl bot added this to Triage in Cluster Observability Aug 8, 2023
@maryliag maryliag moved this from Triage to Quick Win in Cluster Observability Aug 15, 2023
@maryliag maryliag added E-quick-win Likely to be a quick win for someone experienced. good first issue labels Sep 27, 2023
@emilaleksanteri
Copy link
Contributor

Is this one still open and could I take this issue?

emilaleksanteri added a commit to emilaleksanteri/cockroach that referenced this issue Oct 2, 2023
Epic: none
Fixes: cockroachdb#108385

Release note (sql change): make sql.telemetry.query_sampling.max_event_frequency public
craig bot pushed a commit that referenced this issue Oct 3, 2023
111594: sql-telemetry: query_sampling.max_event_frequency public r=maryliag a=emilaleksanteri

Epic: none
Fixes: #108385

Release note (sql change): make max_event_frequency public for public documentation

111638: kvfollowerreadsccl: use `SystemVisible` for `kv.closed_timestamp.propagation_slack` r=erikgrinaker a=erikgrinaker

**changefeedccl: don't use `ALTER TENANT ALL` for closed timestamp setting**

This is no longer necessary with the `SystemVisible` class.

**kvfollowerreadsccl: use `SystemVisible` for `kv.closed_timestamp.propagation_slack`**

It doesn't make any sense to configure this individually per tenant.

Epic: none
Release note: None


Co-authored-by: Emil Lystimaki <emil@circularway.com>
Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>
@craig craig bot closed this as completed in 29e341d Oct 3, 2023
Cluster Observability automation moved this from Quick Win to Done Oct 3, 2023
THardy98 pushed a commit to THardy98/cockroach that referenced this issue Oct 6, 2023
Epic: none
Fixes: cockroachdb#108385

Release note (sql change): make max_event_frequency public for public documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cluster-observability Related to cluster observability C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. good first issue
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants