[craigsoules]

Summary

Fixes yugabyte#35227.

A YSQL tablespace that combined a wildcard placement block with max_num_replicas was accepted by CREATE TABLESPACE, and tables created in it silently got the cluster-level placement (yb-admin rejects the same combination).

Two causes, both fixed here:

1. The wildcard rule was missing from the one validator CREATE TABLESPACE runs. The "every block must be fully qualified when any block has max_num_replicas" rule lived in CatalogManagerUtil::ValidateMaxNumReplicasFields and in yb-admin, but not in TablespaceParser::ValidateReplicationInfo. CREATE/ALTER TABLESPACE only runs the parser's validation (check_satisfiable=false skips the master RPC), so the combination went through. The rule is now in the parser too, with the same error text as the master.

2. An invalid tablespace was indistinguishable from one with no placement. GetYsqlTablespaceInfo validated each tablespace and, on failure, reset it to nullopt -- the same value ReadPgTablespaceInfo uses for a tablespace without replica_placement. Every consumer treated that as "fall back to the cluster config", so CREATE TABLE succeeded with the wrong placement and the load balancer would move replicas to match it. The map (TablespaceIdToReplicationInfoMap) now holds Result<std::optional<ReplicationInfoPB>> and keeps the validation Status:

   • CreateTable fails with the validation error.
   • The load balancer skips tables in an invalid tablespace (surfaced as a kSkipTableLoadBalancing warning) rather than rebalancing them to a placement the user did not ask for. Trade-off: under-replication in such a table is not repaired until the tablespace is fixed.
   • Local transaction table scale-up skips that table but still scales the others, and reports failure so the check retries on the next interval (recovers once the tablespace is fixed).
   • ListTabletServers with a tablespace_id falls back to all tservers for non-NotFound errors, since the result is only a hint.
   • GetTablespaceReplicationInfoWithRetry only refreshes on NotFound, so a persistently invalid tablespace does not trigger a pg_tablespace scan per lookup.
   • Reporting paths (GetTableReplicationInfoWithDefault: expected replica counts in tablet locations, master UI, clone sizing) return the tablespace's declared placement even when it is invalid -- that is what its existing tablets were placed with -- so clients are not told to expect the cluster's replica counts for a table the balancer is deliberately not moving. The map keeps both the parsed placement and the validation status (TablespaceReplicationInfo); only action paths see the error.

Behavior change worth knowing about: AreNodesSafeToTakeDown now fails with the tablespace error if any table lives in an invalid tablespace (it previously used the cluster RF for the quorum calculation). Kept deliberately -- it is a safety check and the error is actionable.

Not addressed here (pre-existing, separate issue): with two read-replica clusters and a tablespace that targets only one of them, ClusterLoadBalancer::GetReadOnlyPlacementFromUuid falls back to read_replicas(0) for the other cluster's pass and the balancer never goes idle. MultipleReadReplicaClustersTablespace had been passing around this by timing; it now waits on the observed placement instead.

Test plan

• tablespace_parser-test (full suite; adds wildcard+max cases incl. the issue's repro, region-level wildcard, read-replica variant, and the wildcard-without-max positive case)
• load_balancer_mocked-test (full suite; GetTableReplicationInfo signature change)
• MasterTest.GetNumTabletReplicasChecksTablespace, MasterTest.GetNumTabletReplicasDefaultsToClusterConfig
• LoadBalancerReadReplicaPlacementPolicyYsqlTest.TablespaceWildcardWithMaxNumReplicas (new: CREATE TABLESPACE rejects the combination)
• TablespaceValidationDisabledTest.TablespaceWildcardWithMaxNumReplicasNotPlaced (new: with --enable_tablespace_validation=false the master still refuses CREATE TABLE, other tablespaces unaffected)
• TablespaceInvalidationTest.TablesInInvalidatedTablespaceAreNotRebalanced (new: existing table whose tablespace becomes invalid is not rebalanced, control table is, tablet locations keep advertising the tablespace's replica counts rather than the cluster's, fixing the tablespace resumes balancing; verified it fails without the load balancer change and without the reporting change)
• TablespaceReadReplicaTest.MultipleReadReplicaClustersTablespace (updated: CREATE TABLE in the ambiguous tablespace now fails instead of using cluster placement)
• MasterTxnStatusCheck.AutoScaleSkipsInvalidTablespace (new: one invalid tablespace does not block scale-up of other local transaction tables; recovers after ALTER TABLESPACE)
• MasterTxnStatusCheck.AutoScaleRetriesWhenTablespaceInfoUnavailable (existing retry behavior preserved)
• lint.sh --rev origin/master clean

[craigsoules]

Binks review on behalf of @craigsoules.

[craigsoules]

🐛 Bug: The new skip path leaves tables in invalid tablespaces at their existing placement, but tablet-location lookups still compute expected replica counts through GetExpectedNumberOfReplicasForTable, which calls GetTableReplicationInfoWithDefault. That helper converts the same non-OK tablespace result into the cluster replication info. For example, after a tablespace with one read-replica placement becomes invalid in a cluster with two read-replica clusters, the load balancer deliberately leaves the table with its existing replica count, while GetTableLocations advertises the cluster's expected replica count. Clients may cache these expected counts and mark the tablet replica set inconsistent, causing repeated metadata refreshes until the tablespace is fixed.

Suggestion: Update expected-replica reporting to avoid GetTableReplicationInfoWithDefault for invalid tablespaces. On validation errors, either omit or derive expected counts from the current tablet config for client-facing location responses, or propagate and handle the validation status in diagnostic callers instead of silently using cluster placement.

---

Binks review on behalf of @craigsoules.

[craigsoules]

Agreed. With the balancer holding the tablespace placement while GetExpectedNumberOfReplicasForTable fell back to the cluster placement, the advertised and enforced counts diverged. The actual client impact is bounded (IsReplicasCountConsistent only drives a re-lookup for consistent-prefix reads, at most once per lookup_cache_refresh_secs), but advertising something other than what we enforce is wrong regardless.

Rather than propagate the error (which would fail GetTableLocations and make the table unreadable) or derive counts from tablet configs, 040f3e7 keeps both the parsed placement and the validation status in the tablespace map (TablespaceReplicationInfo). GetTableReplicationInfoWithDefault -- whose remaining callers are all reporting paths -- now returns the tablespace's declared placement even when invalid, which in the "tablespace became invalid" case is exactly what the existing tablets were placed with; action paths still fail. TablesInInvalidatedTablespaceAreNotRebalanced now asserts the advertised counts are (3, 1), not the cluster's (3, 2) (verified it fails without the change).

---

automated · Claude (pi)