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

server/profiler: remove server.cpu_profile.enabled setting #107717

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

xinhaoz
Copy link
Member

@xinhaoz xinhaoz commented Jul 27, 2023

Cpu profiling can be enabled by setting the cluster setting server.cpu_profile.cpu_usage_combined_threshold. This makes server.cpu_profile.enabled redundant and makes it more difficult and confusing to enable cpu profiling. This commit removes the server.cpu_profile.enabled setting entirely. Note that both jdefault values for the cluster settings set profiling off.

Closes: #102024

Release note (sql change): The cluster setting
server.cpu_profile.enabled has been removed.
server.cpu_profile.cpu_usage_combined_threshold can enable and disable cpu profiling.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@xinhaoz xinhaoz marked this pull request as ready for review July 27, 2023 16:58
@xinhaoz xinhaoz requested a review from a team July 27, 2023 16:58
@xinhaoz xinhaoz requested a review from a team as a code owner July 27, 2023 16:58
@xinhaoz xinhaoz requested review from ericharmeling and a team and removed request for a team July 27, 2023 16:58
Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

Don't forget to mark the setting as retired.

Cpu profiling can be enabled by setting the cluster setting
`server.cpu_profile.cpu_usage_combined_threshold`. This makes
`server.cpu_profile.enabled` redundant and makes it more difficult
and confusing to enable cpu profiling. This commit removes the
`server.cpu_profile.enabled` setting entirely. Note that both
jdefault values for the cluster settings set profiling off.

Closes: cockroachdb#102024

Release note (sql change): The cluster setting
`server.cpu_profile.enabled` has been removed.
`server.cpu_profile.cpu_usage_combined_threshold` can enable and
disable cpu profiling.
@xinhaoz xinhaoz force-pushed the remove-cpu-profile-enabled branch from 1fdd26b to 18b6a6a Compare July 27, 2023 18:28
@xinhaoz
Copy link
Member Author

xinhaoz commented Jul 27, 2023

Don't forget to mark the setting as retired.

Done.

Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ericharmeling)

@xinhaoz
Copy link
Member Author

xinhaoz commented Jul 28, 2023

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 28, 2023

This PR was included in a batch that was canceled, it will be automatically retried

@craig craig bot merged commit 04c91a5 into cockroachdb:master Jul 28, 2023
@craig
Copy link
Contributor

craig bot commented Jul 28, 2023

Build succeeded:

craig bot pushed a commit that referenced this pull request Sep 28, 2023
110377: pkg/server: add an extra server startup guardrail r=knz a=healthy-pod

This code change prevents a SQL server from starting if its minimum supported binary version is greater than the tenant active version.

Release note: None
Epic: CRDB-26691

111143: cli: add --experimental-secondary-cache flag r=RaduBerinde a=itsbilal

This change adds a new `--experimental-secondary-cache` flag for use with `--experimental-shared-storage` to enable the use of a secondary cache to speed up reads of objects in shared storage. This option sets the max cache size for each store using disaggregated storage; for per-store granularity, pebble options can also be used.

Epic: none

Release note: None

111173: sql: add some tests for lookup join when eq cols are key r=yuzefovich a=yuzefovich

These tests are "regression tests" for #108489 if it were to be implemented.

Epic: None

Release note: None

111325: roachtest: add test to ruby-pg blocklist r=fqazi a=andyyang890

Fixes #111116

Release note: None

111384: process-bep-file: create binary for fetching test results from EngFlow r=healthy-pod a=rickystewart

I'll extend this to additionally allow posting GitHub issues.

Part of: DEVINF-871
Epic: CRDB-8308
Release note: None

111433: kv: add shared, replicated, and shared-replicated locks to TestEvaluateBatch r=nvanbenschoten a=nvanbenschoten

Informs #91545.
Informs #100193.

This commit extends TestEvaluateBatch to include shared, replicated, and shared-replicated lock acquisition using Get, Scan, and ReverseScan requests.

Release note: None

111434: roachtest: automatically profile CPU in restore tests r=msbutler a=pavelkalinnikov

See https://www.cockroachlabs.com/docs/v23.1/automatic-cpu-profiler.

NB: the `server.cpu_profile.enabled` setting is not used because it was recently removed in #107717. It should be used if this change is backported.

Touches #111160, #111159
Epic: none
Release note: none

Co-authored-by: healthy-pod <ahmad@cockroachlabs.com>
Co-authored-by: Bilal Akhtar <bilal@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Andy Yang <yang@cockroachlabs.com>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
@xinhaoz xinhaoz deleted the remove-cpu-profile-enabled branch April 1, 2024 17:06
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.

cpuprofiler: remove server.cpu_profile.enabled setting
4 participants