Skip to content

Commit

Permalink
sql: move notifying StatsRefresh of new table to post-commit hook
Browse files Browse the repository at this point in the history
Prior to this change, the StatsRefresher was being notified of a new table
during the execution of the createTable node, before the creating transaction
had committed. Prior to cockroachdb#46170, the StatsRefresher was likely to block on the
intent of the creating transaction. After that change, the StatsRefresher might
not discover the new table because the creating transaction is much more likely
to get pushed further into the future, past the read of the stats refresher.

Release note (bug fix): Fix rare bug where stats were not automatically
generated for a new table.
  • Loading branch information
ajwerner committed Apr 20, 2020
1 parent f16a22f commit 680cd25
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 11 deletions.
19 changes: 19 additions & 0 deletions pkg/sql/conn_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -2148,6 +2148,8 @@ func (ex *connExecutor) txnStateTransitionsApplyWrapper(
res.SetError(newErr)
}
}
ex.notifyStatsRefresherOfNewTables(ex.Ctx())

if err := ex.server.cfg.JobRegistry.Run(
ex.ctxHolder.connCtx,
ex.server.cfg.InternalExecutor,
Expand Down Expand Up @@ -2316,6 +2318,23 @@ func (ex *connExecutor) sessionEventf(ctx context.Context, format string, args .
}
}

// notifyStatsRefresherOfNewTables is call in the post commit hook to inform
// the stats refresher that a new table exists and should have its stats
// collected now.
func (ex *connExecutor) notifyStatsRefresherOfNewTables(ctx context.Context) {
for _, desc := range ex.extraTxnState.tables.getNewTables() {
// The CREATE STATISTICS run for an async CTAS query is initiated by the
// SchemaChanger.
if desc.IsTable() && !desc.IsAs() {
// Initiate a run of CREATE STATISTICS. We use a large number
// for rowsAffected because we want to make sure that stats always get
// created/refreshed here.
ex.planner.execCfg.StatsRefresher.
NotifyMutation(desc.ID, math.MaxInt32 /* rowsAffected */)
}
}
}

// StatementCounters groups metrics for counting different types of
// statements.
type StatementCounters struct {
Expand Down
11 changes: 0 additions & 11 deletions pkg/sql/create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"context"
"fmt"
"go/constant"
"math"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -469,16 +468,6 @@ func (n *createTableNode) startExec(params runParams) error {
}
}

// The CREATE STATISTICS run for an async CTAS query is initiated by the
// SchemaChanger.
if n.n.As() && params.p.autoCommit {
return nil
}

// Initiate a run of CREATE STATISTICS. We use a large number
// for rowsAffected because we want to make sure that stats always get
// created/refreshed here.
params.ExecCfg().StatsRefresher.NotifyMutation(desc.ID, math.MaxInt32 /* rowsAffected */)
return nil
}

Expand Down
9 changes: 9 additions & 0 deletions pkg/sql/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,15 @@ func (tc *TableCollection) getTablesWithNewVersion() []IDVersion {
return tables
}

func (tc *TableCollection) getNewTables() (newTables []*ImmutableTableDescriptor) {
for _, table := range tc.uncommittedTables {
if mut := table.MutableTableDescriptor; mut.IsNewTable() {
newTables = append(newTables, table.ImmutableTableDescriptor)
}
}
return newTables
}

type dbAction bool

const (
Expand Down

0 comments on commit 680cd25

Please sign in to comment.