settings: add functionality for settings override

[RaduBerinde]

This commit adds override functionality to the SettingsWatcher.

We define an OverrideMonitor interface that is used to discover the
overrides and listen for updates. The logic in SettingsWatcher is
updated to return the override whenever there is one set, otherwise
return the current value as normal.

On non-system tenants, an OverrideMonitor implementation will use
new tenant connector APIs to allow the host cluster to override
settings (see the separate RFC).

Release note: None

[cockroach-teamcity]

This change is 

[RaduBerinde]

I don't plan to merge this until the RFC #73349 is merged. I just want to put some of the main implementation features into place so the rest of the work can be easily divided in smaller tasks.

[ajwerner]

The logic here seems fine.

How do we plan on presenting to users whether tenant-rw settings are currently overridden? The logic for SHOW CLUSTER SETTING mostly just reads from the setting value (except for the version setting).

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @RaduBerinde)

---

pkg/server/settingswatcher/settings_watcher.go, line 244 at r1 (raw file):

	defer s.mu.Unlock()

	for key, val := range newOverrides {

should we add something defensive to guard against the version setting being overridden?

[RaduBerinde]

Gasp good question. I can't think of a very easy way to do it that isn't racy. We can add a mutex and an overridden FastIntSet to cluster.Settings and modify the Updater to change both the FastIntSet and the setting value under the mutex. Then we can provide some kind of API to get the value and whether it was overridden under that same mutex.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @dt)

---

pkg/server/settingswatcher/settings_watcher.go, line 244 at r1 (raw file):

Previously, ajwerner wrote…

should we add something defensive to guard against the version setting being overridden?

Added. Though I see now that the updater ignores any Set() calls for the version anyway.

[RaduBerinde]

Rebased, this is ready for review.

[dt]

I'm not terribly concerned about the race potential?

Like, settings have never been transactional -- we show the values as of when we read the settings.SV for each individual row, and they might change while we iterate.

For each name, I'd probably go to the OverridesMonitor first and ask if it is overridden and if so, with what value. If it returns ok and a value, we show the returned value and indicate it is an override, and if not then fall back to reading the setting itself, which will probably not return an overridden value. /2¢

[RaduBerinde]

Yeah, that's definitely an option. Anyway, I will defer that issue for now; I added it to a list of task in a tracking issue.

[dt]

just to confirm my understanding: the expected implementation of OverridesMonitor will internally keep tenant-specific overrides and all-tenant overrides and return the merged map here, right?

[RaduBerinde]

OverridesMonitor will do an RPC through the tenant connector to listen for overrides. The host side of that API will "merge" the overrides. The tenant side won't be able to tell what kind of override it was.

[dt]

Hm. I think we should reconsider that -- but it doesn't really change the API used here either way, so I'm happy to wait on that until the implementation of the monitor and its corresponding host RPC API to litigate that.

(basically I just don't want deleting one all-tenants override to force a read of the every tenant's possible per-tenant override for that setting, so I want them to be stateful and keep track of two layers)

[RaduBerinde]

I agree with that goal. My plan was along the lines of a single process per KV node that watches the entire tenant_settings table and keeps all the information in memory, updating any connected tenants as necessary. I expect that we won't have a ton of entries in that table - thinking we'll have a handful of all-tenant settings and a few one-off tenant-specific settings.

[RaduBerinde]

TFTR!

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)