sql/schema: new system tables causing 22.2 regression in GetZoneConfig

[DrewKimball]

There's a >100% regression in allocations (that means more than double) for the GetZoneConfig benchmark between 22.1 and 22.2. It seems to be related to two system tables that were added in 22.2: SystemPrivilegeTable and SystemExternalConnectionsTable. Possibly this regression is expected, or maybe we're failing to do cache the initialized table descriptors somewhere.

The allocations stem from addSystemDescriptorsToSchema, which calls AddDescriptor for the system tables:

(pprof) list addSystemDescriptorsToSchema
Total: 2.21GB
ROUTINE ======================== github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap.addSystemDescriptorsToSchema in github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap/metadata.go
         0     1.77GB (flat, cum) 80.31% of Total
         .          .    270:func addSystemDescriptorsToSchema(target *MetadataSchema) {
         .          .    271:	// Add system database.
         .          .    272:	target.AddDescriptor(systemschema.SystemDB)
         .          .    273:
         .          .    274:	// Add system config tables.
         .        3MB    275:	target.AddDescriptor(systemschema.NamespaceTable)
         .     6.50MB    276:	target.AddDescriptor(systemschema.DescriptorTable)
         .          .    277:	target.AddDescriptor(systemschema.UsersTable)
         .     6.50MB    278:	target.AddDescriptor(systemschema.ZonesTable)
         .          .    279:	target.AddDescriptor(systemschema.SettingsTable)
         .          .    280:	// Only add the descriptor ID sequence if this is a non-system tenant.
         .          .    281:	// System tenants use the global descIDGenerator key. See #48513.
         .          .    282:	target.AddDescriptorForNonSystemTenant(systemschema.DescIDSequence)
         .          .    283:	target.AddDescriptorForSystemTenant(systemschema.TenantsTable)
         .          .    284:
         .          .    285:	// Add all the other system tables.
         .          .    286:	target.AddDescriptor(systemschema.LeaseTable)
         .        9MB    287:	target.AddDescriptor(systemschema.EventLogTable)
         .          .    288:	target.AddDescriptor(systemschema.RangeEventTable)
         .          .    289:	target.AddDescriptor(systemschema.UITable)
         .          .    290:	target.AddDescriptor(systemschema.JobsTable)
         .          .    291:	target.AddDescriptor(systemschema.WebSessionsTable)
         .          .    292:	target.AddDescriptor(systemschema.RoleOptionsTable)
         .          .    293:
         .          .    294:	// Tables introduced in 2.0, added here for 2.1.
         .          .    295:	target.AddDescriptor(systemschema.TableStatisticsTable)
         .          .    296:	target.AddDescriptor(systemschema.LocationsTable)
         .    21.01MB    297:	target.AddDescriptor(systemschema.RoleMembersTable)
         .          .    298:
         .          .    299:	// The CommentsTable has been introduced in 2.2. It was added here since it
         .          .    300:	// was introduced, but it's also created as a migration for older clusters.
         .          .    301:	target.AddDescriptor(systemschema.CommentsTable)
         .          .    302:	target.AddDescriptor(systemschema.ReportsMetaTable)
         .          .    303:	target.AddDescriptor(systemschema.ReplicationConstraintStatsTable)
         .          .    304:	target.AddDescriptor(systemschema.ReplicationStatsTable)
         .          .    305:	target.AddDescriptor(systemschema.ReplicationCriticalLocalitiesTable)
         .          .    306:	target.AddDescriptor(systemschema.ProtectedTimestampsMetaTable)
         .          .    307:	target.AddDescriptor(systemschema.ProtectedTimestampsRecordsTable)
         .          .    308:
         .          .    309:	// Tables introduced in 20.1.
         .          .    310:
         .          .    311:	target.AddDescriptor(systemschema.StatementBundleChunksTable)
         .          .    312:	target.AddDescriptor(systemschema.StatementDiagnosticsRequestsTable)
         .          .    313:	target.AddDescriptor(systemschema.StatementDiagnosticsTable)
         .          .    314:
         .          .    315:	// Tables introduced in 20.2.
         .          .    316:
         .          .    317:	target.AddDescriptor(systemschema.ScheduledJobsTable)
         .          .    318:	target.AddDescriptor(systemschema.SqllivenessTable)
         .          .    319:	target.AddDescriptor(systemschema.MigrationsTable)
         .          .    320:
         .          .    321:	// Tables introduced in 21.1.
         .          .    322:
         .          .    323:	target.AddDescriptor(systemschema.JoinTokensTable)
         .          .    324:
         .          .    325:	// Tables introduced in 21.2.
         .          .    326:
         .          .    327:	target.AddDescriptor(systemschema.StatementStatisticsTable)
         .    48.05MB    328:	target.AddDescriptor(systemschema.TransactionStatisticsTable)
         .          .    329:	target.AddDescriptor(systemschema.DatabaseRoleSettingsTable)
         .          .    330:	target.AddDescriptorForSystemTenant(systemschema.TenantUsageTable)
         .          .    331:	target.AddDescriptor(systemschema.SQLInstancesTable)
         .          .    332:	target.AddDescriptorForSystemTenant(systemschema.SpanConfigurationsTable)
         .          .    333:
         .          .    334:	// Tables introduced in 22.1.
         .          .    335:
         .   603.40MB    336:	target.AddDescriptorForSystemTenant(systemschema.TenantSettingsTable)
         .          .    337:	target.AddDescriptorForNonSystemTenant(systemschema.SpanCountTable)
         .          .    338:
         .          .    339:	// Tables introduced in 22.2.
         .   512.30MB    340:	target.AddDescriptor(systemschema.SystemPrivilegeTable)
         .   605.40MB    341:	target.AddDescriptor(systemschema.SystemExternalConnectionsTable)
         .          .    342:	target.AddDescriptor(systemschema.RoleIDSequence)
         .          .    343:
         .          .    344:	// Adding a new system table? It should be added here to the metadata schema,
         .          .    345:	// and also created as a migration for older clusters.
         .          .    346:	// If adding a call to AddDescriptor or AddDescriptorForSystemTenant, please

Within AddDescriptor the allocations look like this:

(pprof) list AddDescriptor
Total: 2.21GB
ROUTINE ======================== github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap.(*MetadataSchema).AddDescriptor in github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap/metadata.go
   94.06MB     1.77GB (flat, cum) 80.31% of Total
         .          .     78:	case descpb.InvalidID:
         .          .     79:		if _, isTable := desc.(catalog.TableDescriptor); !isTable {
         .          .     80:			log.Fatalf(context.TODO(), "only system tables may have dynamic IDs, got %T for %s",
         .          .     81:				desc, desc.GetName())
         .          .     82:		}
         .   768.05MB     83:		mut := desc.NewBuilder().BuildCreatedMutable().(*tabledesc.Mutable)
         .          .     84:		mut.ID = ms.allocateID()
         .   953.06MB     85:		desc = mut.ImmutableCopy()
         .          .     86:	default:
         .          .     87:		if ms.ids.Contains(id) {
         .          .     88:			log.Fatalf(context.TODO(), "adding descriptor with duplicate ID: %v", desc)
         .          .     89:		}
         .          .     90:	}
   94.06MB    94.06MB     91:	ms.descs = append(ms.descs, desc)
         .          .     92:}

Jira issue: CRDB-20247

[yuzefovich]

I'm conservatively adding a GA-blocker to this until someone says that this regression is not worth it. cc @ajwerner in case you know something

[ajwerner]

Good news, it was a badly written benchmark.

[ajwerner]

#89661