Skip to content

Commit

Permalink
sql: use in-memory session data to decide what to do in DISCARD
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ajwerner committed Jan 25, 2023
1 parent 66e265e commit 73befd9
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/bench/rttanalysis/testdata/benchmark_expectations
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions pkg/sql/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 73befd9

Please sign in to comment.