Skip to content

Commit

Permalink
opt: refactor used indexes
Browse files Browse the repository at this point in the history
I noticed the unnecessary usage of strings to track index usage in some
CPU/heap profiles. I attempted to eliminate this entirely, but
unfortunately these strings are persisted, making modification of the
type difficult, and I had difficulty following the thread from their
creation all the way to their consumption - I think someone with more
expertise in the observability part of the code base would be required
to untangle this.

So for now I've settled for refactoring the creation of "used indexes" to
avoid using strings until later on during instrumentation. This has the
minor benefit possible de-duplicating the indexes before generating the
strings, if an index is used multiple times in a query.

I've also replaced the expensive usage of `fmt.Sprintf` for generating
the strings to the faster `+` concatenation (benchmarks for this are
at: https://gist.github.com/mgartner/28a9d52e54e91691d0a7617164aa3157).

Release note: None
  • Loading branch information
mgartner committed Oct 10, 2024
1 parent 48aa04c commit 1a90a9b
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 64 deletions.
102 changes: 52 additions & 50 deletions pkg/sql/exec_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,56 +359,58 @@ func (p *planner) maybeLogStatementInternal(
defer releaseSampledQuery(sampledQuery)

*sampledQuery = eventpb.SampledQuery{
CommonSQLExecDetails: execDetails,
SkippedQueries: skippedQueries,
CostEstimate: p.curPlan.instrumentation.costEstimate,
Distribution: p.curPlan.instrumentation.distribution.String(),
PlanGist: p.curPlan.instrumentation.planGist.String(),
SessionID: p.extendedEvalCtx.SessionID.String(),
Database: p.CurrentDatabase(),
StatementID: p.stmt.QueryID.String(),
TransactionID: txnID,
StatementFingerprintID: stmtFingerprintID.String(),
MaxFullScanRowsEstimate: p.curPlan.instrumentation.maxFullScanRows,
TotalScanRowsEstimate: p.curPlan.instrumentation.totalScanRows,
OutputRowsEstimate: p.curPlan.instrumentation.outputRows,
StatsAvailable: p.curPlan.instrumentation.statsAvailable,
NanosSinceStatsCollected: int64(p.curPlan.instrumentation.nanosSinceStatsCollected),
BytesRead: p.curPlan.instrumentation.topLevelStats.bytesRead,
RowsRead: p.curPlan.instrumentation.topLevelStats.rowsRead,
RowsWritten: p.curPlan.instrumentation.topLevelStats.rowsWritten,
InnerJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.InnerJoin]),
LeftOuterJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.LeftOuterJoin]),
FullOuterJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.FullOuterJoin]),
SemiJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.LeftSemiJoin]),
AntiJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.LeftAntiJoin]),
IntersectAllJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.IntersectAllJoin]),
ExceptAllJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.ExceptAllJoin]),
HashJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.HashJoin]),
CrossJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.CrossJoin]),
IndexJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.IndexJoin]),
LookupJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.LookupJoin]),
MergeJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.MergeJoin]),
InvertedJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.InvertedJoin]),
ApplyJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.ApplyJoin]),
ZigZagJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.ZigZagJoin]),
ContentionNanos: queryLevelStats.ContentionTime.Nanoseconds(),
Regions: queryLevelStats.Regions,
SQLInstanceIDs: queryLevelStats.SQLInstanceIDs,
KVNodeIDs: queryLevelStats.KVNodeIDs,
UsedFollowerRead: queryLevelStats.UsedFollowerRead,
NetworkBytesSent: queryLevelStats.NetworkBytesSent,
MaxMemUsage: queryLevelStats.MaxMemUsage,
MaxDiskUsage: queryLevelStats.MaxDiskUsage,
KVBytesRead: queryLevelStats.KVBytesRead,
KVPairsRead: queryLevelStats.KVPairsRead,
KVRowsRead: queryLevelStats.KVRowsRead,
KvTimeNanos: queryLevelStats.KVTime.Nanoseconds(),
KvGrpcCalls: queryLevelStats.KVBatchRequestsIssued,
NetworkMessages: queryLevelStats.NetworkMessages,
CpuTimeNanos: queryLevelStats.CPUTime.Nanoseconds(),
IndexRecommendations: indexRecs,
Indexes: p.curPlan.instrumentation.indexesUsed,
CommonSQLExecDetails: execDetails,
SkippedQueries: skippedQueries,
CostEstimate: p.curPlan.instrumentation.costEstimate,
Distribution: p.curPlan.instrumentation.distribution.String(),
PlanGist: p.curPlan.instrumentation.planGist.String(),
SessionID: p.extendedEvalCtx.SessionID.String(),
Database: p.CurrentDatabase(),
StatementID: p.stmt.QueryID.String(),
TransactionID: txnID,
StatementFingerprintID: stmtFingerprintID.String(),
MaxFullScanRowsEstimate: p.curPlan.instrumentation.maxFullScanRows,
TotalScanRowsEstimate: p.curPlan.instrumentation.totalScanRows,
OutputRowsEstimate: p.curPlan.instrumentation.outputRows,
StatsAvailable: p.curPlan.instrumentation.statsAvailable,
NanosSinceStatsCollected: int64(p.curPlan.instrumentation.nanosSinceStatsCollected),
BytesRead: p.curPlan.instrumentation.topLevelStats.bytesRead,
RowsRead: p.curPlan.instrumentation.topLevelStats.rowsRead,
RowsWritten: p.curPlan.instrumentation.topLevelStats.rowsWritten,
InnerJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.InnerJoin]),
LeftOuterJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.LeftOuterJoin]),
FullOuterJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.FullOuterJoin]),
SemiJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.LeftSemiJoin]),
AntiJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.LeftAntiJoin]),
IntersectAllJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.IntersectAllJoin]),
ExceptAllJoinCount: int64(p.curPlan.instrumentation.joinTypeCounts[descpb.ExceptAllJoin]),
HashJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.HashJoin]),
CrossJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.CrossJoin]),
IndexJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.IndexJoin]),
LookupJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.LookupJoin]),
MergeJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.MergeJoin]),
InvertedJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.InvertedJoin]),
ApplyJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.ApplyJoin]),
ZigZagJoinCount: int64(p.curPlan.instrumentation.joinAlgorithmCounts[exec.ZigZagJoin]),
ContentionNanos: queryLevelStats.ContentionTime.Nanoseconds(),
Regions: queryLevelStats.Regions,
SQLInstanceIDs: queryLevelStats.SQLInstanceIDs,
KVNodeIDs: queryLevelStats.KVNodeIDs,
UsedFollowerRead: queryLevelStats.UsedFollowerRead,
NetworkBytesSent: queryLevelStats.NetworkBytesSent,
MaxMemUsage: queryLevelStats.MaxMemUsage,
MaxDiskUsage: queryLevelStats.MaxDiskUsage,
KVBytesRead: queryLevelStats.KVBytesRead,
KVPairsRead: queryLevelStats.KVPairsRead,
KVRowsRead: queryLevelStats.KVRowsRead,
KvTimeNanos: queryLevelStats.KVTime.Nanoseconds(),
KvGrpcCalls: queryLevelStats.KVBatchRequestsIssued,
NetworkMessages: queryLevelStats.NetworkMessages,
CpuTimeNanos: queryLevelStats.CPUTime.Nanoseconds(),
IndexRecommendations: indexRecs,
// TODO(mgartner): Use a slice of struct{uint64, uint64} instead of
// converting to strings.
Indexes: p.curPlan.instrumentation.indexesUsed.Strings(),
ScanCount: int64(p.curPlan.instrumentation.scanCounts[exec.ScanCount]),
ScanWithStatsCount: int64(p.curPlan.instrumentation.scanCounts[exec.ScanWithStatsCount]),
ScanWithStatsForecastCount: int64(p.curPlan.instrumentation.scanCounts[exec.ScanWithStatsForecastCount]),
Expand Down
6 changes: 4 additions & 2 deletions pkg/sql/executor_statement_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,10 @@ func (ex *connExecutor) recordStatementSummary(
EndTime: phaseTimes.GetSessionPhaseTime(sessionphase.PlannerStartExecStmt).Add(svcLatRaw),
FullScan: fullScan,
ExecStats: queryLevelStats,
Indexes: planner.instrumentation.indexesUsed,
Database: planner.SessionData().Database,
// TODO(mgartner): Use a slice of struct{uint64, uint64} instead of
// converting to strings.
Indexes: planner.instrumentation.indexesUsed.Strings(),
Database: planner.SessionData().Database,
}

stmtFingerprintID, err :=
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/instrumentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/idxrecommendations"
"github.com/cockroachdb/cockroach/pkg/sql/isql"
"github.com/cockroachdb/cockroach/pkg/sql/opt/exec"
"github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder"
"github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain"
"github.com/cockroachdb/cockroach/pkg/sql/opt/indexrec"
"github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder"
Expand Down Expand Up @@ -225,7 +226,7 @@ type instrumentationHelper struct {
scanCounts [exec.NumScanCountTypes]int

// indexesUsed list the indexes used in the query with format tableID@indexID.
indexesUsed []string
indexesUsed execbuilder.IndexesUsed

// schemachangerMode indicates which schema changer mode was used to execute
// the query.
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/opt/exec/execbuilder/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ go_library(
"//pkg/sql/sqlerrors",
"//pkg/sql/sqltelemetry",
"//pkg/sql/types",
"//pkg/util",
"//pkg/util/buildutil",
"//pkg/util/encoding",
"//pkg/util/errorutil",
Expand Down
38 changes: 37 additions & 1 deletion pkg/sql/opt/exec/execbuilder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package execbuilder

import (
"context"
"slices"
"strconv"
"time"

"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
Expand Down Expand Up @@ -168,7 +170,41 @@ type Builder struct {
IsANSIDML bool

// IndexesUsed list the indexes used in query with the format tableID@indexID.
IndexesUsed []string
IndexesUsed
}

// IndexesUsed is a list of indexes used in a query.
type IndexesUsed struct {
indexes []struct {
tableID cat.StableID
indexID cat.StableID
}
}

// add adds the given index to the list, if it is not already present.
func (iu *IndexesUsed) add(tableID, indexID cat.StableID) {
s := struct {
tableID cat.StableID
indexID cat.StableID
}{tableID, indexID}
if !slices.Contains(iu.indexes, s) {
iu.indexes = append(iu.indexes, s)
}
}

// Strings returns a slice of strings with the format tableID@indexID for each
// index in the list.
//
// TODO(mgartner): Use a slice of struct{uint64, uint64} instead of converting
// to strings.
func (iu *IndexesUsed) Strings() []string {
res := make([]string, len(iu.indexes))
const base = 10
for i, u := range iu.indexes {
res[i] = strconv.FormatUint(uint64(u.tableID), base) + "@" +
strconv.FormatUint(uint64(u.indexID), base)
}
return res
}

// New constructs an instance of the execution node builder using the
Expand Down
15 changes: 6 additions & 9 deletions pkg/sql/opt/exec/execbuilder/relational.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/buildutil"
"github.com/cockroachdb/cockroach/pkg/util/encoding"
"github.com/cockroachdb/cockroach/pkg/util/errorutil"
Expand Down Expand Up @@ -753,7 +752,7 @@ func (b *Builder) buildScan(scan *memo.ScanExpr) (_ execPlan, outputCols colOrdM
return execPlan{}, colOrdMap{},
errors.AssertionFailedf("expected inverted index scan to have a constraint")
}
b.IndexesUsed = util.CombineUnique(b.IndexesUsed, []string{fmt.Sprintf("%d@%d", tab.ID(), idx.ID())})
b.IndexesUsed.add(tab.ID(), idx.ID())

// Save if we planned a full (large) table/index scan on the builder so that
// the planner can be made aware later. We only do this for non-virtual
Expand Down Expand Up @@ -2294,7 +2293,7 @@ func (b *Builder) buildIndexJoin(
// TODO(radu): the distsql implementation of index join assumes that the input
// starts with the PK columns in order (#40749).
pri := tab.Index(cat.PrimaryIndex)
b.IndexesUsed = util.CombineUnique(b.IndexesUsed, []string{fmt.Sprintf("%d@%d", tab.ID(), pri.ID())})
b.IndexesUsed.add(tab.ID(), pri.ID())
keyCols := make([]exec.NodeColumnOrdinal, pri.KeyColumnCount())
for i := range keyCols {
keyCols[i], err = getNodeColumnOrdinal(inputCols, join.Table.ColumnID(pri.Column(i).Ordinal()))
Expand Down Expand Up @@ -2672,7 +2671,7 @@ func (b *Builder) buildLookupJoin(

tab := md.Table(join.Table)
idx := tab.Index(join.Index)
b.IndexesUsed = util.CombineUnique(b.IndexesUsed, []string{fmt.Sprintf("%d@%d", tab.ID(), idx.ID())})
b.IndexesUsed.add(tab.ID(), idx.ID())

locking, err := b.buildLocking(join.Table, join.Locking)
if err != nil {
Expand Down Expand Up @@ -2852,7 +2851,7 @@ func (b *Builder) buildInvertedJoin(
md := b.mem.Metadata()
tab := md.Table(join.Table)
idx := tab.Index(join.Index)
b.IndexesUsed = util.CombineUnique(b.IndexesUsed, []string{fmt.Sprintf("%d@%d", tab.ID(), idx.ID())})
b.IndexesUsed.add(tab.ID(), idx.ID())

prefixEqCols := make([]exec.NodeColumnOrdinal, len(join.PrefixKeyCols))
for i, c := range join.PrefixKeyCols {
Expand Down Expand Up @@ -2994,10 +2993,8 @@ func (b *Builder) buildZigzagJoin(
rightTable := md.Table(join.RightTable)
leftIndex := leftTable.Index(join.LeftIndex)
rightIndex := rightTable.Index(join.RightIndex)
b.IndexesUsed = util.CombineUnique(b.IndexesUsed,
[]string{fmt.Sprintf("%d@%d", leftTable.ID(), leftIndex.ID())})
b.IndexesUsed = util.CombineUnique(b.IndexesUsed,
[]string{fmt.Sprintf("%d@%d", rightTable.ID(), rightIndex.ID())})
b.IndexesUsed.add(leftTable.ID(), leftIndex.ID())
b.IndexesUsed.add(rightTable.ID(), rightIndex.ID())

leftEqCols := make([]exec.TableColumnOrdinal, len(join.LeftEqCols))
rightEqCols := make([]exec.TableColumnOrdinal, len(join.RightEqCols))
Expand Down

0 comments on commit 1a90a9b

Please sign in to comment.