Skip to content

storage: add blob-rewrite compaction cluster settings#148782

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
jbowens:blobrw-clustersettings
Jun 25, 2025
Merged

storage: add blob-rewrite compaction cluster settings#148782
craig[bot] merged 1 commit intocockroachdb:masterfrom
jbowens:blobrw-clustersettings

Conversation

@jbowens
Copy link
Copy Markdown
Contributor

@jbowens jbowens commented Jun 25, 2025

Add two new cluster settings for configuring blob-rewrite compactions.

Epic: CRDB-20379
Release note: None

@jbowens jbowens requested a review from a team as a code owner June 25, 2025 01:14
@jbowens jbowens requested a review from RaduBerinde June 25, 2025 01:14
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Jun 25, 2025

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

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

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@annrpom annrpom 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 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde)


-- commits line 7 at r1:
nit: should we document this new cluster setting in the release note

@jbowens jbowens force-pushed the blobrw-clustersettings branch from 03189b7 to fdb52e4 Compare June 25, 2025 02:08
Copy link
Copy Markdown
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @annrpom and @jbowens)


pkg/storage/pebble.go line 384 at r2 (raw file):

		settings.DurationWithMinimum(0),
	)
	valueSeparationCompactionGarbageThreshold = settings.RegisterFloatSetting(

[nit] Consider making this an integer percent (it's more user-friendly).


pkg/storage/pebble.go line 387 at r2 (raw file):

		settings.SystemVisible,
		"storage.value_separation.compaction_garbage_threshold",
		"the max garbage threshold configures the maximum fraction of unreferenced values"+

[nit] This talks about the fraction of values but we're really talking about the fraction in terms of bytes (btw the TargetGarbageRatio description is similar). Maybe
"fraction of space wasted by unreferenced values which triggers blob-file rewrite compactions (1.0 disables these compactions)."


pkg/storage/pebble.go line 388 at r2 (raw file):

		"storage.value_separation.compaction_garbage_threshold",
		"the max garbage threshold configures the maximum fraction of unreferenced values"+
			" that may be garbage without performing blob-file rewrite compactions. When"+

[nit] no space between When and unreferenced. I'd standardize the space at the end everywhere


pkg/storage/pebble.go line 392 at r2 (raw file):

			" triggered to reclaim disk space.",
		1.0, /* default; disables blob-file rewrites */
		settings.FloatInRange(0.0, 1.0),

[nit] Maybe set 0.1 as the minimum

@jbowens jbowens force-pushed the blobrw-clustersettings branch from fdb52e4 to e2fd131 Compare June 25, 2025 12:46
Copy link
Copy Markdown
Contributor Author

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

TFTRS!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @annrpom)

@jbowens jbowens force-pushed the blobrw-clustersettings branch from e2fd131 to 0285522 Compare June 25, 2025 14:28
Add two new cluster settings for configuring blob-rewrite compactions.

Epic: CRDB-20379
Release note (ops change): Add cluster settings for configuring blob file
rewrite compactions.
@jbowens jbowens force-pushed the blobrw-clustersettings branch from 0285522 to feb3805 Compare June 25, 2025 15:04
@jbowens
Copy link
Copy Markdown
Contributor Author

jbowens commented Jun 25, 2025

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig Bot commented Jun 25, 2025

@craig craig Bot merged commit 1ded868 into cockroachdb:master Jun 25, 2025
22 checks passed
@jbowens
Copy link
Copy Markdown
Contributor Author

jbowens commented Jun 25, 2025

blathers backport release-25.3

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants