Skip to content

Commit

Permalink
sql/lease: support skipping descriptor validation lease renewals
Browse files Browse the repository at this point in the history
Fixes: #95764

Previously, when renewing a lease we would fetch dependent
descriptors for cross-descriptor validation. This could
lead to perpetual transaction retries, in the schema where
there are tons of dependencies between descriptors since
lease renewals would happen in a high-priority transaction.
Additionally, this incurs additional latency for lease renewal.
To address this, this patch disables cross-validation by default for
lease renewals.

Epic: none
Release note (bug fix): Disable cross-descriptor validation on lease renewal,
which can be problematic when there are lots of descriptors with lots
of foreign key references, in which cases, the cross-reference validation could
starve schema changes.
  • Loading branch information
fqazi committed Feb 24, 2023
1 parent ad3c5d5 commit 063cbfe
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
4 changes: 4 additions & 0 deletions pkg/base/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ const (
// DefaultDescriptorLeaseRenewalTimeout is the default time
// before a lease expires when acquisition to renew the lease begins.
DefaultDescriptorLeaseRenewalTimeout = time.Minute

// DefaultLeaseRenewalCrossValidate is the default setting for if
// we should validate descriptors on lease renewals.
DefaultLeaseRenewalCrossValidate = false
)

// DefaultCertsDirectory is the default value for the cert directory flag.
Expand Down
2 changes: 1 addition & 1 deletion pkg/bench/rttanalysis/testdata/benchmark_expectations
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,5 @@ exp,benchmark
12,Truncate/truncate_2_column_2_rows
1,VirtualTableQueries/select_crdb_internal.invalid_objects_with_1_fk
1,VirtualTableQueries/select_crdb_internal.tables_with_1_fk
11,VirtualTableQueries/virtual_table_cache_with_point_lookups
9,VirtualTableQueries/virtual_table_cache_with_point_lookups
7,VirtualTableQueries/virtual_table_cache_with_schema_change
19 changes: 17 additions & 2 deletions pkg/sql/catalog/lease/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ var LeaseRenewalDuration = settings.RegisterDurationSetting(
"controls the default time before a lease expires when acquisition to renew the lease begins",
base.DefaultDescriptorLeaseRenewalTimeout)

// LeaseRenewalCrossValidate controls if cross validation should be done during
// lease renewal.
var LeaseRenewalCrossValidate = settings.RegisterBoolSetting(
settings.TenantWritable,
"sql.catalog.descriptor_lease_renewal_cross_validation.enabled",
"controls if cross validation should be done during lease renewal",
base.DefaultLeaseRenewalCrossValidate)

func (s storage) leaseRenewalTimeout() time.Duration {
return LeaseRenewalDuration.Get(&s.settings.SV)
}
Expand All @@ -99,6 +107,10 @@ func (s storage) jitteredLeaseDuration() time.Duration {
2*jitterFraction*rand.Float64()))
}

func (s storage) crossValidateDuringRenewal() bool {
return LeaseRenewalCrossValidate.Get(&s.settings.SV)
}

// acquire a lease on the most recent version of a descriptor. If the lease
// cannot be obtained because the descriptor is in the process of being dropped
// or offline (currently only applicable to tables), the error will be of type
Expand Down Expand Up @@ -148,7 +160,6 @@ func (s storage) acquire(
// a monotonically increasing expiration.
expiration = minExpiration.Add(int64(time.Millisecond), 0)
}

desc, err = s.mustGetDescriptorByID(ctx, txn, id)
if err != nil {
return err
Expand Down Expand Up @@ -291,13 +302,17 @@ func (s storage) mustGetDescriptorByID(
return nil, err
}
desc := c.LookupDescriptor(id)
validationLevel := catalog.ValidationLevelSelfOnly
if s.crossValidateDuringRenewal() {
validationLevel = validate.ImmutableRead
}
vd := catkv.NewCatalogReaderBackedValidationDereferencer(cr, txn, nil /* dvmpMaybe */)
ve := validate.Validate(
ctx,
s.settings.Version.ActiveVersion(ctx),
vd,
catalog.ValidationReadTelemetry,
validate.ImmutableRead,
validationLevel,
desc,
)
if err := ve.CombinedError(); err != nil {
Expand Down

0 comments on commit 063cbfe

Please sign in to comment.