sql: gcjob and opt need ZoneConfig which is not available on tenants

[tbg]

See

cockroach/pkg/sql/gcjob/refresh_statuses.go

Line 95 in c37f28f

zoneCfg, placeholder, _, err := sql.ZoneConfigHook(cfg, uint32(tableID))

The way we've designed clearing up after a DROP TABLE is fundamentally incompatible with a hard split between KV and SQL, unless we add a way for the SQL layer to figure out the zone config for a given table from the KV backend. The other way to do this is to push the responsibility for cleaning up into KV, which is a bigger change but is "better" in the sense that it doesn't rely on the SQL tenant process running (which often won't be the case after the TTL).

We should be able to hard-code a value here instead for the time being.

Similar in opt:

cockroach/pkg/sql/opt/xform/coster.go

Line 754 in 74741e6

adjustment := 1.0 - localityMatchScore(idx.Zone(), c.locality)

We have a pretty hard dependency on reasoning about the physical location of data. Here there seems no way around it, we need to be able to resolve table ID to localities, and this becomes a blocker the moment we go geo-distributed.

cc @nvanbenschoten

[blathers-crl]

Hi @tbg, please add a C-ategory label to your issue. Check out the label system docs.

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

[tbg]

The way the dependency goes here is weird enough to mention. The ZoneConfigHook lives in sql:

cockroach/pkg/sql/zone_config.go

Lines 153 to 174 in 15c7371

	// ZoneConfigHook returns the zone config for the object with id using the
	// cached system config. If keySuffix is within a subzone, the subzone's config
	// is returned instead. The bool is set to true when the value returned is
	// cached.
	func ZoneConfigHook(
	cfg *config.SystemConfig, id uint32,
	) (*zonepb.ZoneConfig, *zonepb.ZoneConfig, bool, error) {
	getKey := func(key roachpb.Key) (*roachpb.Value, error) {
	return cfg.GetValue(key), nil
	}
	zoneID, zone, _, placeholder, err := getZoneConfig(
	id, getKey, false /* getInheritedDefault */)
	if errors.Is(err, errNoZoneConfigApplies) {
	return nil, nil, true, nil
	} else if err != nil {
	return nil, nil, false, err
	}
	if err = completeZoneConfig(zone, zoneID, getKey); err != nil {
	return nil, nil, false, err
	}
	return zone, placeholder, true, nil
	}

The zone configs are stored in the system tenant:

cockroach/pkg/config/keys.go

Lines 24 to 27 in 6927a10

	// MakeZoneKey returns the key for id's entry in the system.zones table.
	func MakeZoneKey(id uint32) roachpb.Key {
	return keys.SystemSQLCodec.ZoneKey(id)
	}

So roughly what we want is some way for non-system tenants to get (transactional?) read-only access to the zones covering their keyspace.

This all seems doable. I wouldn't guess that we need transactional access here, at least not for gcjob or opt.

[tbg]

cc @nvanbenschoten since this came up tangentially in #49459

[tbg]

Just discussed: we make it optional now (bypassing any uses of it), but down the road we should allow the tenant to read the system configs that pertain to it.

[tbg]

We need to figure something out though. system.zones is keyed on the system tenant's table descriptors. Exceptions (liveness ranges, etc) get hard-coded descriptor IDs. This doesn't extend to tenants. Need to discuss more and come up with a way in which this will work once we need it.