Skip to content

Commit

Permalink
Merge #96767
Browse files Browse the repository at this point in the history
96767: spanconfig: fix assertion that ensures split at tenant boundary r=ecwall a=arulajmani

We added an assertion in #96014 to ensure that full reconciliation always preserves a split point at the start of the tenant's keyspace. The comment that introduces the assertion explains such a span config record should be present, however, makes the assertion that such a record is the first span config record for the tenant.

There's no real reason to make the "first record" assertion here. In fact, that assumption isn't correct when system span configurations are in play, as they sort before regular span configurations. I noticed this broke `TestBackupRestoreTenantSettings`, causing a panic in the reconciler, while workong on an unrelated patch.

This patch relaxes the "first record" assertion. We're happy as long as it's present somewhere.

Release note: None
Epic: None

Co-authored-by: Arul Ajmani <[email protected]>
  • Loading branch information
craig[bot] and arulajmani committed Feb 8, 2023
2 parents 7ee61fb + 394918d commit ff67a4b
Showing 1 changed file with 14 additions and 6 deletions.
20 changes: 14 additions & 6 deletions pkg/spanconfig/spanconfigreconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ func (f *fullReconciler) reconcile(
}

if !f.codec.ForSystemTenant() {
found := false
tenantPrefixKey := f.codec.TenantPrefix()
// We want to ensure tenant ranges do not straddle tenant boundaries. As
// such, a full reconciliation should always include a SpanConfig where the
Expand All @@ -309,17 +310,24 @@ func (f *fullReconciler) reconcile(
// tenant. Also, sql.CreateTenantRecord relies on such a SpanConfigs
// existence to ensure the same thing for newly created tenants.
if err := storeWithLatestSpanConfigs.Iterate(func(record spanconfig.Record) error {
if record.GetTarget().IsSystemTarget() {
return nil // skip over system span configurations,
}
spanConfigStartKey := record.GetTarget().GetSpan().Key
if tenantPrefixKey.Compare(spanConfigStartKey) != 0 {
return errors.AssertionFailedf(
"tenant prefix key %q does not match first span config start key %q",
tenantPrefixKey, spanConfigStartKey,
)
if tenantPrefixKey.Compare(spanConfigStartKey) == 0 {
found = true
return iterutil.StopIteration()
}
return iterutil.StopIteration()
return nil
}); err != nil {
return nil, hlc.Timestamp{}, err
}

if !found {
return nil, hlc.Timestamp{}, errors.AssertionFailedf(
"did not find split point at the start of the tenant's keyspace during full reconciliation",
)
}
}

return storeWithLatestSpanConfigs, readTimestamp, nil
Expand Down

0 comments on commit ff67a4b

Please sign in to comment.