Skip to content

Commit

Permalink
Merge #85634 #85760
Browse files Browse the repository at this point in the history
85634: sql: Add retry reason and node ids to execution_insights r=j82w a=j82w

Adding the retry reason and node ids to execution_insights

part of #81024

Release note (sql change): Adding last_retry_reason and 
exec_node_ids columns to
crdb_internal.node_execution_insights

85760: sql: lower number of users created for role id migration r=ajwerner a=RichardJCai

Test was previously flakey due to timeouts (takes roughly 2 minutes).
Test completes in 15s now.

Release note: None

Co-authored-by: j82w <jwilley@cockroachlabs.com>
Co-authored-by: richardjcai <caioftherichard@gmail.com>
  • Loading branch information
3 people committed Aug 9, 2022
3 parents 9c840c6 + 9f2a2ea + 2d4458d commit 24906bd
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 8 deletions.
19 changes: 18 additions & 1 deletion pkg/sql/crdb_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -6311,7 +6311,9 @@ CREATE TABLE crdb_internal.node_execution_insights (
rows_read INT8 NOT NULL,
rows_written INT8 NOT NULL,
priority FLOAT NOT NULL,
retries INT8 NOT NULL
retries INT8 NOT NULL,
last_retry_reason STRING,
exec_node_ids INT[] NOT NULL
);`,
populate: func(ctx context.Context, p *planner, db catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) (err error) {
p.extendedEvalCtx.statsProvider.IterateInsights(ctx, func(
Expand All @@ -6329,6 +6331,19 @@ CREATE TABLE crdb_internal.node_execution_insights (
return
}

execNodeIDs := tree.NewDArray(types.Int)
for _, nodeID := range insight.Statement.Nodes {
if errNodeID := execNodeIDs.Append(tree.NewDInt(tree.DInt(nodeID))); errNodeID != nil {
err = errors.CombineErrors(err, errNodeID)
return
}
}

autoRetryReason := tree.DNull
if insight.Statement.AutoRetryReason != "" {
autoRetryReason = tree.NewDString(insight.Statement.AutoRetryReason)
}

err = errors.CombineErrors(err, addRow(
tree.NewDString(hex.EncodeToString(insight.Session.ID.GetBytes())),
tree.NewDUuid(tree.DUuid{UUID: insight.Transaction.ID}),
Expand All @@ -6348,6 +6363,8 @@ CREATE TABLE crdb_internal.node_execution_insights (
tree.NewDInt(tree.DInt(insight.Statement.RowsWritten)),
tree.NewDFloat(tree.DFloat(insight.Transaction.UserPriority)),
tree.NewDInt(tree.DInt(insight.Statement.Retries)),
autoRetryReason,
execNodeIDs,
))
})
return err
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/executor_statement_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ func (ex *connExecutor) recordStatementSummary(
SessionID: ex.sessionID,
StatementID: planner.stmt.QueryID,
AutoRetryCount: automaticRetryCount,
AutoRetryReason: ex.state.mu.autoRetryReason,
RowsAffected: rowsAffected,
ParseLatency: parseLat,
PlanLatency: planLat,
Expand Down
8 changes: 6 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/create_statements
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,9 @@ CREATE TABLE crdb_internal.node_execution_insights (
rows_read INT8 NOT NULL,
rows_written INT8 NOT NULL,
priority FLOAT8 NOT NULL,
retries INT8 NOT NULL
retries INT8 NOT NULL,
last_retry_reason STRING NULL,
exec_node_ids INT8[] NOT NULL
) CREATE TABLE crdb_internal.node_execution_insights (
session_id STRING NOT NULL,
txn_id UUID NOT NULL,
Expand All @@ -947,7 +949,9 @@ CREATE TABLE crdb_internal.node_execution_insights (
rows_read INT8 NOT NULL,
rows_written INT8 NOT NULL,
priority FLOAT8 NOT NULL,
retries INT8 NOT NULL
retries INT8 NOT NULL,
last_retry_reason STRING NULL,
exec_node_ids INT8[] NOT NULL
) {} {}
CREATE TABLE crdb_internal.node_inflight_trace_spans (
trace_id INT8 NOT NULL,
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/sqlstats/insights/insights.proto
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ message Statement {
int64 rows_read = 13;
int64 rows_written = 14;
int64 retries = 15;
string auto_retry_reason = 16;
// Nodes is the ordered list of nodes ids on which the statement was executed.
repeated int64 nodes = 17;
}

message Insight {
Expand Down
9 changes: 8 additions & 1 deletion pkg/sql/sqlstats/ssmemstorage/ss_mem_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ func (s *Container) RecordStatement(
}
}

var autoRetryReason string
if value.AutoRetryReason != nil {
autoRetryReason = value.AutoRetryReason.Error()
}

s.outliersRegistry.ObserveStatement(value.SessionID, &insights.Statement{
ID: value.StatementID,
FingerprintID: stmtFingerprintID,
Expand All @@ -179,13 +184,15 @@ func (s *Container) RecordStatement(
StartTime: value.StartTime,
EndTime: value.EndTime,
FullScan: value.FullScan,
User: value.SessionData.User().SQLIdentifier(),
User: value.SessionData.User().Normalized(),
ApplicationName: value.SessionData.ApplicationName,
Database: value.SessionData.Database,
PlanGist: value.PlanGist,
Retries: int64(value.AutoRetryCount),
AutoRetryReason: autoRetryReason,
RowsRead: value.RowsRead,
RowsWritten: value.RowsWritten,
Nodes: value.Nodes,
})

return stats.ID, nil
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/sqlstats/ssprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ type RecordedStmtStats struct {
StatementID clusterunique.ID
TransactionID uuid.UUID
AutoRetryCount int
AutoRetryReason error
RowsAffected int
ParseLatency float64
PlanLatency float64
Expand Down
9 changes: 5 additions & 4 deletions pkg/upgrade/upgrades/role_id_migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func runTestRoleIDMigration(t *testing.T, numUsers int) {
wg.Add(1)
// Create other users in parallel while the migration is happening.
go func() {
for i := 0; i < 1000; i++ {
for i := 0; i < 500; i++ {
tdb.Exec(t, fmt.Sprintf(`CREATE USER parallel_user_creation_%d`, i))
}
wg.Done()
Expand Down Expand Up @@ -168,7 +168,7 @@ func runTestRoleIDMigration(t *testing.T, numUsers int) {
{"admin", "", "true", "2"},
{"root", "", "false", "1"},
})
tdb.CheckQueryResults(t, `SELECT user_id > 100000 FROM system.users WHERE username = 'testuser_last'`, [][]string{
tdb.CheckQueryResults(t, `SELECT user_id > 10000 FROM system.users WHERE username = 'testuser_last'`, [][]string{
{"true"},
})
}
Expand Down Expand Up @@ -204,9 +204,10 @@ func TestRoleIDMigration100User(t *testing.T) {
runTestRoleIDMigration(t, 100)
}

func TestRoleIDMigration100000Users(t *testing.T) {
func TestRoleIDMigration15000Users(t *testing.T) {
skip.UnderStress(t)
runTestRoleIDMigration(t, 100000)
// 15000 is 1.5x the batch size used in the migration.
runTestRoleIDMigration(t, 15000)
}

func getDeprecatedSystemUsersTable() *descpb.TableDescriptor {
Expand Down

0 comments on commit 24906bd

Please sign in to comment.