From 73befd934d7931704ac068edf09acf3522e975d6 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Wed, 25 Jan 2023 17:06:04 -0500 Subject: [PATCH] sql: use in-memory session data to decide what to do in DISCARD In #86246 we introduced logic to discard schemas when running DISCARD ALL and DISCARD TEMP. This logic did expensive kv operations unconditionally; if the session knew it had never created a temporary schema, we'd still fetch all databases and proceed to search all databases for a temp schema. This is very expensive. Fixes: #95864 Release note (performance improvement): In 22.2, we introduced support for `DISCARD TEMP` and made `DISCARD ALL` actually discard temp tables. This implementation ran expensive logic to discover temp schemas rather than consulting in-memory data structures. As a result, `DISCARD ALL`, which is issued regularly by connection pools, became an expensive operation when it should be cheap. This problem is now resolved. --- .../rttanalysis/testdata/benchmark_expectations | 2 +- pkg/sql/discard.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/bench/rttanalysis/testdata/benchmark_expectations b/pkg/bench/rttanalysis/testdata/benchmark_expectations index 6c67c8842d70..970ae2872ae3 100644 --- a/pkg/bench/rttanalysis/testdata/benchmark_expectations +++ b/pkg/bench/rttanalysis/testdata/benchmark_expectations @@ -33,7 +33,7 @@ exp,benchmark 13,CreateRole/create_role_with_no_options 18,"Discard/DISCARD_ALL,_1_tables_in_1_db" 21,"Discard/DISCARD_ALL,_2_tables_in_2_dbs" -9,"Discard/DISCARD_ALL,_no_tables" +1,"Discard/DISCARD_ALL,_no_tables" 10,DropDatabase/drop_database_0_tables 11,DropDatabase/drop_database_1_table 11,DropDatabase/drop_database_2_tables diff --git a/pkg/sql/discard.go b/pkg/sql/discard.go index ec6fa86016e6..4561170a6f35 100644 --- a/pkg/sql/discard.go +++ b/pkg/sql/discard.go @@ -84,14 +84,26 @@ func (n *discardNode) startExec(params runParams) error { } func deleteTempTables(ctx context.Context, p *planner) error { + // If this session has no temp schemas, then there is nothing to do here. + // This is the common case. + if len(p.SessionData().DatabaseIDToTempSchemaID) == 0 { + return nil + } codec := p.execCfg.Codec descCol := p.Descriptors() + // Note: grabbing all the databases here is somewhat suspect. It appears + // that the logic related to maintaining the set of database temp schemas + // is somewhat incomplete, so there can be temp schemas in the sessiondata + // map which don't exist any longer. allDbDescs, err := descCol.GetAllDatabaseDescriptors(ctx, p.Txn()) if err != nil { return err } g := p.byNameGetterBuilder().MaybeGet() for _, db := range allDbDescs { + if _, ok := p.SessionData().DatabaseIDToTempSchemaID[uint32(db.GetID())]; !ok { + continue + } sc, err := g.Schema(ctx, db, p.TemporarySchemaName()) if err != nil { return err