storage: add cluster setting to use old compaction eligibility #152356
Conversation
In cockroachdb/pebble#5187 we altered the default compaction picking logic that determines whether a level is elgible for a default compaction. We added a knob to allow the configuration of this value at runtime in case the change has some unforeseen impact on some workloads. This commit connects this knob to a new cluster setting. This cluster setting is just an escape hatch that we don't anticipate needing. Epic: none Release note: none
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=95b789b1d594ac6b74f377aa7555d8d98d63240d&v=4){kind=avatar}
|
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. |
|
This change is |
|
I wonder if we should make the knob more general - "revert to deprecated compaction heuristics" and each value is a version (documented as matching compaction heuristics in a specific crdb version) |
|
I thought about making something more general, but I feel like it starts to actually get more ambiguous because not all behavior changes will be gated by cluster settings and we'll eventually start removing support for old behavior. I'd rather keep this as an undocumented setting that we don't even attempt to explain to customers, because we don't anticipate customers should ever touch it. |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @xxmplus)
In cockroachdb/pebble#5187 we altered the default compaction picking logic that determines whether a level is elgible for a default compaction. We added a knob to allow the configuration of this value at runtime in case the change has some unforeseen impact on some workloads. This commit connects this knob to a new cluster setting. This cluster setting is just an escape hatch that we don't anticipate needing.
Epic: none
Release note: none