sql,spanconfig: attribute usage costs to tenant-settable fields

[irfansharif]

Is your feature request related to a problem? Please describe.

Secondary tenants can set zone configs now (#67679). Before rolling it out, we ought to consider what fields should be settable by them, and/or what fields are considered by KV. The full set of settable parameters are:

cockroach/pkg/roachpb/span_config.proto

Lines 121 to 123 in 0e7034c

	// SpanConfig holds the configuration that applies to a given keyspan. It is a
	// superset of the fields found in zonepb.zone.proto.
	message SpanConfig {

There are a few reasons to tread carefully here (non-exhaustive):

• We currently cost tenants based on logical bytes. Given secondary tenant zone configs allow users to configure the replication factor, there's nothing preventing them from replicating data N-ways where N is the number of nodes in the host cluster. This could be expensive for the host cluster.
• Zone configs with our current costing make it possible for tenants to bus large amounts of data for free. By reconfiguring where the data is located (through constraints), it's possible to induce substantial egress.
• Setting low range_min_bytes could cause a large number of range split for the tenant. It's worth noting that this isn't limited by the mechanisms proposed in kvserver: limit the number of kv spans configurable by a tenant #70555.
• Setting super low or super high GC TTLs could cause KV to either scan for garbage frequently, or keep a lot more garbage data around (again, for free).

Describe the solution you'd like

If we're able to cost+rate limit things precisely, some of the aforementioned concerns go away. We could also start off with ignoring all fields down in KV and selectively accepting some of them as we gain confidence. That would however present as non-conformant ranges for secondary tenants, which could be bad-enough UX to instead disallow setting zone configs entirely.

Describe alternatives you've considered

See above.

Additional context

#67679 also powers MR abstractions for secondary tenants. Whatever we do has implications for MR usability/billing etc. We could also explore what it would mean allowing only MR set configs (though we lack the plumbing necessary today to distinguish between MR-set configs and user-set ones). Even for MR-set configs, some of the costing considerations above still apply.

+cc @ajstorm, @ajwerner, @nvanbenschoten, @arulajmani.

Jira issue: CRDB-13537

Epic CRDB-18604

[blathers-crl]

Hi @irfansharif, please add branch-* labels to identify which branch(es) this release-blocker affects.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[irfansharif]

I'm going to mark this as a GA-blocker to ensure at the very least we ship with #78299. That PR does not fully address what this issue talks about (cost attribution for all the work you can induce through zone configs), but it does provide safeguards for 22.1. Once that lands, we can get rid of the GA-blocker label. Given we've layered PTS/tenant backups on top of zone configs, perhaps we want to explicitly allow setting the protected-timestamp fields? If so, we should also disable this by default:

cockroach/pkg/sql/exec_util.go

Lines 200 to 207 in 4fc43c8

	// TODO(irfansharif): This should be a tenant-readonly setting, possible after
	// the work for #73349 is completed.
	var secondaryTenantZoneConfigsEnabled = settings.RegisterBoolSetting(
	settings.TenantWritable,
	secondaryTenantsZoneConfigsEnabledSettingName,
	"allow secondary tenants to set zone configurations; does not affect the system tenant",
	true,
	)

[arulajmani]

Once that lands, we can get rid of the GA-blocker label. Given we've layered PTS/tenant backups on top of zone configs, perhaps we want to explicitly allow setting the protected-timestamp fields?

Given we've layered PTS/tenant backups on top of span configs, doesn't this entail a bit more than what the PR does? We can't blanket turn of the KVAccessor for all secondary tenants and have PTS support for secondary tenants -- we might instead need to validate span configs being upserted all have default values for their fields bar the PTS field. All this assuming #70555 is fixed as well.

[irfansharif]

Removing the GA-blocker label now that we've merged #79097. It disallows tenants directly modification zone config fields, gated by an operator controlled cluster setting. Protected timestamps that use span configs underneath the hood still work, and in environments where we don't care about tenant cost attribution, enabling the cluster setting will give secondary tenants full access to all fields. The cluster setting also allows us to enable the infrastructure on a tenant-by-tenant basis. The top-level issue talks about how use of zone configs could today induce work that isn't cost attributed back to the tenant. In environments where we care about that, we'll still want to do more work here. It's feasible also that we never expose zone configs directly in those environments, or only a subset (like replication factor).

[irfansharif]

+cc @knz's #85954 (for some reason the github cross linking thing didn't appear above). That RFC talks about whether the ability to set certain zone config fields or not can be modeled as "capabilities" that a tenant has/doesn't.