settings: more guidance #111586
settings: more guidance #111586
Conversation
|
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 |
pkg/settings/setting.go
Outdated
| // One use cases is to enable optimizations in the KV client side of | ||
| // virtual cluster RPCs. For example, if the storage layer uses a | ||
| // setting to decide the size of a response, making the setting | ||
| // visible enables the client in virtual cluster ocde to |
| // system-visible settings, can be overridden using ALTER VIRTUAL | ||
| // CLUSTER SET CLUSTER SETTING. This can be used during e.g. | ||
| // troubleshooting, to allow changing the observed value for a | ||
| // virtual cluster without a code change. |
There was a problem hiding this comment.
maybe add one more sentence here:
A setting that is expected to be overridden in regular operation to to control application-level behavior is still an application setting, and should use the application class; system-visible is for use by settings that control the system layer but need to be visible, rather than for settings that control the application layer but are not intended to be modified by it; the latter should use an application-class setting, as it controls application behavior, and then rely on an override to prevent modification from within the virtual cluster.
Release note: None
knz
force-pushed
the
20231002-setting-advice2
branch
from
9ae3b6d to
268153b
Compare
|
bors r=dt |
|
Build succeeded: |
Epic: CRDB-6671
As requested by @dt here.
Release note: None