From b3713c785ea39338dccaf38f82de41a5e0a9b272 Mon Sep 17 00:00:00 2001 From: Marius Posta <marius@cockroachlabs.com> Date: Wed, 25 Jan 2023 10:05:23 -0500 Subject: [PATCH] validate: use immutable descriptors only The descriptor validation logic will accept any implementation of catalog.Descriptor be it mutable or immutable, it doesn't care. However, using mutable implementations can have a significant performance impact especially in the case of tables, where every column or index or constraint lookup will lead to the cache being regenerated for the whole descriptor. This commit fixes this by having validate.Validate replace any mutable descriptor instances it encounters with immutable copies. This doesn't change anything except performance. Fixes #95827. Release note: None --- pkg/sql/catalog/internal/validate/validate.go | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/pkg/sql/catalog/internal/validate/validate.go b/pkg/sql/catalog/internal/validate/validate.go index 616dd23ed605..3d1d834bccbe 100644 --- a/pkg/sql/catalog/internal/validate/validate.go +++ b/pkg/sql/catalog/internal/validate/validate.go @@ -58,6 +58,15 @@ func Validate( targetLevel catalog.ValidationLevel, descriptors ...catalog.Descriptor, ) catalog.ValidationErrors { + for i, d := range descriptors { + // Replace mutable descriptors with immutable copies. Validation is + // read-only in any case, and using immutables can have a significant + // impact on performance when validating tables due to columns, indexes, + // and so forth being cached. + if mut, ok := d.(catalog.MutableDescriptor); ok { + descriptors[i] = mut.ImmutableCopy() + } + } vea := validationErrorAccumulator{ ValidationTelemetry: telemetry, targetLevel: targetLevel, @@ -409,9 +418,17 @@ func (cs *collectorState) getMissingDescs( return nil, err } for _, desc := range resps { - if desc != nil { - cs.vdg.descriptors[desc.GetID()] = desc + if desc == nil { + continue + } + if mut, ok := desc.(catalog.MutableDescriptor); ok { + // Replace mutable descriptors with immutable copies. Validation is + // read-only in any case, and using immutables can have a significant + // impact on performance when validating tables due to columns, indexes, + // and so forth being cached. + desc = mut.ImmutableCopy() } + cs.vdg.descriptors[desc.GetID()] = desc } return resps, nil }