settings: rename and better document the setting classes

[knz]

All commits except the last are from #110758.
Epic: CRDB-6671
Informs #58445.

TLDR: "TenantReadOnly" -> "SystemVisible"; "TenantWritable" -> "ApplicationLevel".

Review guidance: the only manual change is in pkg/settings/setting.go. Everything else was automated
search-replace.

See the new/extended explanatory comments in pkg/settings/setting.go for details.

// Class describes the scope of a setting under cluster
// virtualization.
//
// Guidelines for choosing a class:
//
//   - Make sure to read the descriptions below carefully to
//     understand the differences in semantics.
//
//   - Rules of thumb:
//
//     1. if an end-user should be able to modify the setting in
//     CockroachCloud Serverless, AND different end-users should be
//     able to use different values, the setting should have class
//     ApplicationLevel.
//
//     2. if a setting should only be controlled by SREs in
//     CockroachCloud Serverless, OR a single value must apply to
//     multiple tenants (virtual clusters) simultaneously, the setting
//     must not use class ApplicationLevel.
//
//     3. if and only if a setting relevant to the KV/storage layer
//     and whose value is shared across all virtual cluster is ever
//     accessed by code in the application layer (SQL execution or
//     HTTP handlers), use SystemVisible.
//
//     4. in other cases, use SystemOnly.
type Class int8

const (
  // SystemOnly settings are specific to the KV/storage layer and
  // cannot be accessed from application layer code (in particular not
  // from SQL layer nor HTTP handlers).
  //
  // As a rule of thumb, use this class if:
  //
  //   - the setting may be accessed from the shared KV/storage layer;
  //
  //   - AND, the setting should only be controllable by SREs (not
  //     end-users) in CockroachCloud Serverless, AND a single value
  //     must apply to all virtual clusters simultaneously;
  //
  //     (If this part of the condition does not hold, consider
  //     ApplicationLevel instead.)
  //
  //   - AND, its value is never needed in the application layer (i.e.
  //     it is not read from SQL code, HTTP handlers or other code
  //     that would run in SQL pods in CC Serverless).
  //
  //     (If this part of the condition does not hold, consider
  //     SystemVisible instead.)
  SystemOnly Class = iota

  // SystemVisible settings are specific to the KV/storage layer and
  // are also visible to virtual clusters.
  //
  // As a rule of thumb, use this class if:
  //
  //   - the setting may be accessed from the shared KV/storage layer;
  //
  //   - AND the setting should only be controllable by SREs (not
  //     end-users) in CockroachCloud Serverless, AND a single value
  //     must apply to all virtual clusters simultaneously;
  //
  //     (If this part of the condition does not hold, consider
  //     ApplicationLevel instead.)
  //
  //   - AND, its value is sometimes needed in the application layer
  //     (i.e. it may be read from SQL code, HTTP handlers or other
  //     code that could run in SQL pods in CC Serverless).
  //
  //     (If this part of the condition does not hold, consider
  //     SystemOnly instead.)
  SystemVisible

  // ApplicationLevel settings are readable and can optionally be
  // modified by virtual clusters.
  //
  // As a rule of thumb, use this class if:
  //
  //   - the setting is never accessed by the shared KV/storage layer;
  //
  //   - AND, any of the following holds:
  //
  //     - end-users should legitimately be able to modify
  //       the setting in CockroachCloud Serverless;
  //
  //       (If this part of the condition does not hold, but the other
  //       part of the condition below does hold, consider still using
  //       ApplicationLevel and force an override using ALTER VIRTUAL
  //       CLUSTER SET CLUSTER SETTING. This makes the
  //       ApplicationLevel setting effectively read-only from the
  //       virtual cluster's perspective, because system overrides
  //       cannot be modified by the virtual cluster.)
  //
  //     - OR, different virtual clusters should be able to
  //       use different values for the setting.
  //
  //       (If this part of the condition does not hold, consider
  //       SystemOnly or SystemVisible instead.)
  ApplicationLevel

Release note: None

[mdlinville]

Release note (cluster virtualization): Cluster settings are now grouped into 3 classes with clearer descriptions:

• "system only" for settings only accessible to the shared KV/storage
  layer and thus only available to the operator.
• "system visible" for storage-level settings that can be viewed from
  virtual clusters.
• "application level" for settings that can be viewed from virtual
  clusters, may be different between virtual clusters, or can be
  customized by end-users.

Looks like the actual labels are SystemOnly, SystemVisible, and ApplicationLevel, right? (with that spacing and casing)

[knz]

Looks like the actual labels are SystemOnly, SystemVisible, and ApplicationLevel, right? (with that spacing and casing)

These labels are internal to the source code and not user-facing, which is why they are not mentioned in the release note. Only the (auto-generated) table with settings will mention the classes, and the doc writer can choose which labels to use to generate the table (using the --class-labels parameter to cockroach gen setting-list).

[ajstorm]

Nit: You might want to word this as "if and only if" to avoid splitting the "if" and "only then" part between so many words. Additionally, you may want to split the last sentence out to its own bullet, as it's a pretty important point that's somewhat buried right now. Finally instead of "non-ApplicationLevel", can we say "System-level"?

[knz]

You might want to word this as "if and only if" to avoid splitting the "if" and "only then" part between so many words.

Done.

you may want to split the last sentence out to its own bullet

Done.

Finally instead of "non-ApplicationLevel", can we say "System-level"?

I think we should include both. The 3 points so far only mentioned ApplicationLevel, so I believe the text here should refer to it primarily.

[stevendanna]

Overall, I think this is moving us in the right direction. Just one small suggestion.

[stevendanna]

I wonder if we should provide some more guidance here. Namely, most settings should be ApplicationLevel if they aren't in the KV/storage layer.

[knz]

Done.

[stevendanna]

We have some settings like those in the tenantcostclient that sort of violate this as written. In the current documentation, it looks like we should have made those ApplicationLevel and CC should have set an override.

[knz]

Very astute observation. I am very keen to have that convo with the CC team. But I feel that this convo will only be productive after we settle on the guidance.

[RaduBerinde]

I agree. I don't think we should rely on external process to set those overrides though - we could set them ourselves by default (in a permanent upgrade).

[RaduBerinde]

Why should it be rarely used? It seems that all the settings related to KV and below (all the way down to Pebble settings) belong here, of which there are many.

[knz]

ok i'll soften that up.

[RaduBerinde]

I like the new naming and guidance, nicely done!

[knz]

bors r=ajstorm,stevendanna,dt,RaduBerinde

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[erikgrinaker]

Thanks for the cleanup and improvements here!

ApplicationLevel settings are readable and can optionally be modified by virtual clusters.

Consider pointing out that virtual clusters never see the system setting, these are entirely independent per tenant.

AND a single value must apply to all virtual clusters simultaneously

Consider pointing out that it is still possible for the operator to set per-tenant values for a setting.

[knz]

thanks for the extra feedback, let's discuss first.