From b291909f855d29e4613b44a266cd3c8fd21c0ad2 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Thu, 2 May 2019 10:13:15 -0400 Subject: [PATCH] sql: only increment statement counters upon success Before this PR, statement counters were incremented before executing a SQL statement. This is problematic because it means that the counters include statements which fail during execution. This commit adds an additional set of statement counters which track successfully executed statements which use the old metric name. The reason the new behavior uses the old name is to retain historical data in the UI for these charts after an upgrade. This does unfortunately mean that the definition of this metric will change in telemetry where the old definition will have a new name. This is both probably okay from an analytics perspective (and maybe even preferable) and can be mitigated with some fancy SQL. Release note (admin ui change): Only include successfully executed statements in the statement counters. --- pkg/sql/conn_executor.go | 96 ++++++-- pkg/sql/conn_executor_exec.go | 48 +++- pkg/sql/exec_util.go | 209 +++++++++++++----- pkg/sql/metric_test.go | 76 ++++--- pkg/sql/metric_util_test.go | 28 +-- pkg/sql/pgwire/server.go | 6 +- pkg/sql/txn_restart_test.go | 4 +- .../nodeGraphs/dashboards/overview.tsx | 2 +- .../containers/nodeGraphs/dashboards/sql.tsx | 2 +- 9 files changed, 329 insertions(+), 142 deletions(-) diff --git a/pkg/sql/conn_executor.go b/pkg/sql/conn_executor.go index dbcbb59e6509..99aff6045dd4 100644 --- a/pkg/sql/conn_executor.go +++ b/pkg/sql/conn_executor.go @@ -263,8 +263,15 @@ type Metrics struct { // for metrics registration. EngineMetrics EngineMetrics - // StatementCounters contains metrics for statements. - StatementCounters StatementCounters + // StartedStatementCounters contains metrics for statements initiated by + // users. These metrics count user-initiated operations, regardless of + // success (in particular, TxnCommitCount is the number of COMMIT statements + // attempted, not the number of transactions that successfully commit). + StartedStatementCounters StatementCounters + + // ExecutedStatementCounters contains metrics for successfully executed + // statements. + ExecutedStatementCounters StatementCounters } // NewServer creates a new Server. Start() needs to be called before the Server @@ -304,7 +311,8 @@ func makeMetrics(internal bool) Metrics { TxnAbortCount: metric.NewCounter(getMetricMeta(MetaTxnAbort, internal)), FailureCount: metric.NewCounter(getMetricMeta(MetaFailure, internal)), }, - StatementCounters: makeStatementCounters(internal), + StartedStatementCounters: makeStartedStatementCounters(internal), + ExecutedStatementCounters: makeExecutedStatementCounters(internal), } } @@ -364,7 +372,7 @@ func (s *Server) GetExecutorConfig() *ExecutorConfig { // and an error is returned if this validation fails. // stmtBuf: The incoming statement for the new connExecutor. // clientComm: The interface through which the new connExecutor is going to -// produce results for the client. +// produce results for the client. // memMetrics: The metrics that statements executed on this connection will // contribute to. func (s *Server) SetupConn( @@ -2118,10 +2126,7 @@ func (ex *connExecutor) sessionEventf(ctx context.Context, format string, args . } // StatementCounters groups metrics for counting different types of -// statements. These metrics count user-initiated operations, -// regardless of success (in particular, TxnCommitCount is the number -// of COMMIT statements attempted, not the number of transactions that -// successfully commit). +// statements. type StatementCounters struct { // QueryCount includes all statements and it is therefore the sum of // all the below metrics. @@ -2154,24 +2159,69 @@ type StatementCounters struct { MiscCount telemetry.CounterWithMetric } -func makeStatementCounters(internal bool) StatementCounters { +func makeStartedStatementCounters(internal bool) StatementCounters { + return StatementCounters{ + TxnBeginCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaTxnBeginStarted, internal)), + TxnCommitCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaTxnCommitStarted, internal)), + TxnRollbackCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaTxnRollbackStarted, internal)), + SavepointCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaSavepointStarted, internal)), + RestartSavepointCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaRestartSavepointStarted, internal)), + ReleaseRestartSavepointCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaReleaseRestartSavepointStarted, internal)), + RollbackToRestartSavepointCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaRollbackToRestartSavepointStarted, internal)), + SelectCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaSelectStarted, internal)), + UpdateCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaUpdateStarted, internal)), + InsertCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaInsertStarted, internal)), + DeleteCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaDeleteStarted, internal)), + DdlCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaDdlStarted, internal)), + MiscCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaMiscStarted, internal)), + QueryCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaQueryStarted, internal)), + } +} + +func makeExecutedStatementCounters(internal bool) StatementCounters { return StatementCounters{ - TxnBeginCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaTxnBegin, internal)), - TxnCommitCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaTxnCommit, internal)), - TxnRollbackCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaTxnRollback, internal)), - SavepointCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaSavepoint, internal)), - RestartSavepointCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaRestartSavepoint, internal)), + TxnBeginCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaTxnBeginExecuted, internal)), + TxnCommitCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaTxnCommitExecuted, internal)), + TxnRollbackCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaTxnRollbackExecuted, internal)), + SavepointCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaSavepointExecuted, internal)), + RestartSavepointCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaRestartSavepointExecuted, internal)), ReleaseRestartSavepointCount: telemetry.NewCounterWithMetric( - getMetricMeta(MetaReleaseRestartSavepoint, internal)), + getMetricMeta(MetaReleaseRestartSavepointExecuted, internal)), RollbackToRestartSavepointCount: telemetry.NewCounterWithMetric( - getMetricMeta(MetaRollbackToRestartSavepoint, internal)), - SelectCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaSelect, internal)), - UpdateCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaUpdate, internal)), - InsertCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaInsert, internal)), - DeleteCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaDelete, internal)), - DdlCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaDdl, internal)), - MiscCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaMisc, internal)), - QueryCount: telemetry.NewCounterWithMetric(getMetricMeta(MetaQuery, internal)), + getMetricMeta(MetaRollbackToRestartSavepointExecuted, internal)), + SelectCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaSelectExecuted, internal)), + UpdateCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaUpdateExecuted, internal)), + InsertCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaInsertExecuted, internal)), + DeleteCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaDeleteExecuted, internal)), + DdlCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaDdlExecuted, internal)), + MiscCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaMiscExecuted, internal)), + QueryCount: telemetry.NewCounterWithMetric( + getMetricMeta(MetaQueryExecuted, internal)), } } diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go index 05e2cd313828..3ac59e5f93d7 100644 --- a/pkg/sql/conn_executor_exec.go +++ b/pkg/sql/conn_executor_exec.go @@ -134,7 +134,12 @@ func (ex *connExecutor) recordFailure() { func (ex *connExecutor) execStmtInOpenState( ctx context.Context, stmt Statement, pinfo *tree.PlaceholderInfo, res RestrictedCommandResult, ) (retEv fsm.Event, retPayload fsm.EventPayload, retErr error) { - ex.incrementStmtCounter(stmt) + ex.incrementStartedStmtCounter(stmt) + defer func() { + if retErr == nil && !payloadHasError(retPayload) { + ex.incrementExecutedStmtCounter(stmt) + } + }() os := ex.machine.CurState().(stateOpen) var timeoutTicker *time.Timer @@ -947,10 +952,15 @@ func (ex *connExecutor) beginTransactionTimestampsAndReadMode( // stateOpen, at each point its results will also be flushed. func (ex *connExecutor) execStmtInNoTxnState( ctx context.Context, stmt Statement, -) (fsm.Event, fsm.EventPayload) { +) (_ fsm.Event, payload fsm.EventPayload) { switch s := stmt.AST.(type) { case *tree.BeginTransaction: - ex.incrementStmtCounter(stmt) + ex.incrementStartedStmtCounter(stmt) + defer func() { + if !payloadHasError(payload) { + ex.incrementExecutedStmtCounter(stmt) + } + }() pri, err := priorityToProto(s.Modes.UserPriority) if err != nil { return ex.makeErrEvent(err, s) @@ -1096,8 +1106,13 @@ func (ex *connExecutor) execStmtInAbortedState( // Everything but COMMIT/ROLLBACK causes errors. ROLLBACK is treated like COMMIT. func (ex *connExecutor) execStmtInCommitWaitState( stmt Statement, res RestrictedCommandResult, -) (fsm.Event, fsm.EventPayload) { - ex.incrementStmtCounter(stmt) +) (ev fsm.Event, payload fsm.EventPayload) { + ex.incrementStartedStmtCounter(stmt) + defer func() { + if !payloadHasError(payload) { + ex.incrementExecutedStmtCounter(stmt) + } + }() switch stmt.AST.(type) { case *tree.CommitTransaction, *tree.RollbackTransaction: // Reply to a rollback with the COMMIT tag, by analogy to what we do when we @@ -1105,8 +1120,8 @@ func (ex *connExecutor) execStmtInCommitWaitState( res.ResetStmtType((*tree.CommitTransaction)(nil)) return eventTxnFinish{}, eventTxnFinishPayload{commit: true} default: - ev := eventNonRetriableErr{IsCommit: fsm.False} - payload := eventNonRetriableErrPayload{ + ev = eventNonRetriableErr{IsCommit: fsm.False} + payload = eventNonRetriableErrPayload{ err: sqlbase.NewTransactionCommittedError(), } return ev, payload @@ -1291,8 +1306,23 @@ func (ex *connExecutor) handleAutoCommit( return ev, payload } -func (ex *connExecutor) incrementStmtCounter(stmt Statement) { - ex.metrics.StatementCounters.incrementCount(ex, stmt.AST) +// incrementStartedStmtCounter increments the appropriate started +// statement counter for stmt's type. +func (ex *connExecutor) incrementStartedStmtCounter(stmt Statement) { + ex.metrics.StartedStatementCounters.incrementCount(ex, stmt.AST) +} + +// incrementExecutedStmtCounter increments the appropriate executed +// statement counter for stmt's type. +func (ex *connExecutor) incrementExecutedStmtCounter(stmt Statement) { + ex.metrics.ExecutedStatementCounters.incrementCount(ex, stmt.AST) +} + +// payloadHasError returns true if the passed payload implements +// payloadWithError. +func payloadHasError(payload fsm.EventPayload) bool { + _, hasErr := payload.(payloadWithError) + return hasErr } // validateSavepointName validates that it is that the provided ident diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index 6d1e0001c67b..ee3f3273525d 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -193,36 +193,6 @@ const metricsSampleInterval = 10 * time.Second // Fully-qualified names for metrics. var ( - MetaTxnBegin = metric.Metadata{ - Name: "sql.txn.begin.count", - Help: "Number of SQL transaction BEGIN statements", - Measurement: "SQL Statements", - Unit: metric.Unit_COUNT, - } - MetaTxnCommit = metric.Metadata{ - Name: "sql.txn.commit.count", - Help: "Number of SQL transaction COMMIT statements", - Measurement: "SQL Statements", - Unit: metric.Unit_COUNT, - } - MetaTxnAbort = metric.Metadata{ - Name: "sql.txn.abort.count", - Help: "Number of SQL transaction abort errors", - Measurement: "SQL Statements", - Unit: metric.Unit_COUNT, - } - MetaTxnRollback = metric.Metadata{ - Name: "sql.txn.rollback.count", - Help: "Number of SQL transaction ROLLBACK statements", - Measurement: "SQL Statements", - Unit: metric.Unit_COUNT, - } - MetaSelect = metric.Metadata{ - Name: "sql.select.count", - Help: "Number of SQL SELECT statements", - Measurement: "SQL Statements", - Unit: metric.Unit_COUNT, - } MetaSQLExecLatency = metric.Metadata{ Name: "sql.exec.latency", Help: "Latency of SQL statement execution", @@ -277,69 +247,188 @@ var ( Measurement: "Latency", Unit: metric.Unit_NANOSECONDS, } - MetaUpdate = metric.Metadata{ + MetaTxnAbort = metric.Metadata{ + Name: "sql.txn.abort.count", + Help: "Number of SQL transaction abort errors", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaFailure = metric.Metadata{ + Name: "sql.failure.count", + Help: "Number of statements resulting in a planning or runtime error", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + + // Below are the metadata for the statement started counters. + + MetaQueryStarted = metric.Metadata{ + Name: "sql.query.started.count", + Help: "Number of SQL queries started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaTxnBeginStarted = metric.Metadata{ + Name: "sql.txn.begin.started.count", + Help: "Number of SQL transaction BEGIN statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaTxnCommitStarted = metric.Metadata{ + Name: "sql.txn.commit.started.count", + Help: "Number of SQL transaction COMMIT statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaTxnRollbackStarted = metric.Metadata{ + Name: "sql.txn.rollback.started.count", + Help: "Number of SQL transaction ROLLBACK statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaSelectStarted = metric.Metadata{ + Name: "sql.select.started.count", + Help: "Number of SQL SELECT statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaUpdateStarted = metric.Metadata{ + Name: "sql.update.started.count", + Help: "Number of SQL UPDATE statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaInsertStarted = metric.Metadata{ + Name: "sql.insert.started.count", + Help: "Number of SQL INSERT statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaDeleteStarted = metric.Metadata{ + Name: "sql.delete.started.count", + Help: "Number of SQL DELETE statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaSavepointStarted = metric.Metadata{ + Name: "sql.savepoint.started.count", + Help: "Number of SQL SAVEPOINT statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaRestartSavepointStarted = metric.Metadata{ + Name: "sql.restart_savepoint.started.count", + Help: "Number of `SAVEPOINT cockroach_restart` statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaReleaseRestartSavepointStarted = metric.Metadata{ + Name: "sql.restart_savepoint.release.started.count", + Help: "Number of `RELEASE SAVEPOINT cockroach_restart` statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaRollbackToRestartSavepointStarted = metric.Metadata{ + Name: "sql.restart_savepoint.rollback.started.count", + Help: "Number of `ROLLBACK TO SAVEPOINT cockroach_restart` statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaDdlStarted = metric.Metadata{ + Name: "sql.ddl.started.count", + Help: "Number of SQL DDL statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaMiscStarted = metric.Metadata{ + Name: "sql.misc.started.count", + Help: "Number of other SQL statements started", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + + // Below are the metadata for the statement executed counters. + MetaQueryExecuted = metric.Metadata{ + Name: "sql.query.count", + Help: "Number of SQL queries executed", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaTxnBeginExecuted = metric.Metadata{ + Name: "sql.txn.begin.count", + Help: "Number of SQL transaction BEGIN statements successfully executed", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaTxnCommitExecuted = metric.Metadata{ + Name: "sql.txn.commit.count", + Help: "Number of SQL transaction COMMIT statements successfully executed", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaTxnRollbackExecuted = metric.Metadata{ + Name: "sql.txn.rollback.count", + Help: "Number of SQL transaction ROLLBACK statements successfully executed", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaSelectExecuted = metric.Metadata{ + Name: "sql.select.count", + Help: "Number of SQL SELECT statements successfully executed", + Measurement: "SQL Statements", + Unit: metric.Unit_COUNT, + } + MetaUpdateExecuted = metric.Metadata{ Name: "sql.update.count", - Help: "Number of SQL UPDATE statements", + Help: "Number of SQL UPDATE statements successfully executed", Measurement: "SQL Statements", Unit: metric.Unit_COUNT, } - MetaInsert = metric.Metadata{ + MetaInsertExecuted = metric.Metadata{ Name: "sql.insert.count", - Help: "Number of SQL INSERT statements", + Help: "Number of SQL INSERT statements successfully executed", Measurement: "SQL Statements", Unit: metric.Unit_COUNT, } - MetaDelete = metric.Metadata{ + MetaDeleteExecuted = metric.Metadata{ Name: "sql.delete.count", - Help: "Number of SQL DELETE statements", + Help: "Number of SQL DELETE statements successfully executed", Measurement: "SQL Statements", Unit: metric.Unit_COUNT, } - MetaSavepoint = metric.Metadata{ + MetaSavepointExecuted = metric.Metadata{ Name: "sql.savepoint.count", - Help: "Number of SQL SAVEPOINT statements", + Help: "Number of SQL SAVEPOINT statements successfully executed", Measurement: "SQL Statements", Unit: metric.Unit_COUNT, } - MetaRestartSavepoint = metric.Metadata{ + MetaRestartSavepointExecuted = metric.Metadata{ Name: "sql.restart_savepoint.count", - Help: "Number of `SAVEPOINT cockroach_restart` statements", + Help: "Number of `SAVEPOINT cockroach_restart` statements successfully executed", Measurement: "SQL Statements", Unit: metric.Unit_COUNT, } - MetaReleaseRestartSavepoint = metric.Metadata{ + MetaReleaseRestartSavepointExecuted = metric.Metadata{ Name: "sql.restart_savepoint.release.count", - Help: "Number of `RELEASE SAVEPOINT cockroach_restart` statements", + Help: "Number of `RELEASE SAVEPOINT cockroach_restart` statements successfully executed", Measurement: "SQL Statements", Unit: metric.Unit_COUNT, } - MetaRollbackToRestartSavepoint = metric.Metadata{ + MetaRollbackToRestartSavepointExecuted = metric.Metadata{ Name: "sql.restart_savepoint.rollback.count", - Help: "Number of `ROLLBACK TO SAVEPOINT cockroach_restart` statements", + Help: "Number of `ROLLBACK TO SAVEPOINT cockroach_restart` statements successfully executed", Measurement: "SQL Statements", Unit: metric.Unit_COUNT, } - MetaDdl = metric.Metadata{ + MetaDdlExecuted = metric.Metadata{ Name: "sql.ddl.count", - Help: "Number of SQL DDL statements", + Help: "Number of SQL DDL statements successfully executed", Measurement: "SQL Statements", Unit: metric.Unit_COUNT, } - MetaMisc = metric.Metadata{ + MetaMiscExecuted = metric.Metadata{ Name: "sql.misc.count", - Help: "Number of other SQL statements", - Measurement: "SQL Statements", - Unit: metric.Unit_COUNT, - } - MetaQuery = metric.Metadata{ - Name: "sql.query.count", - Help: "Number of SQL queries", - Measurement: "SQL Statements", - Unit: metric.Unit_COUNT, - } - MetaFailure = metric.Metadata{ - Name: "sql.failure.count", - Help: "Number of statements resulting in a planning or runtime error", + Help: "Number of other SQL statements successfully executed", Measurement: "SQL Statements", Unit: metric.Unit_COUNT, } diff --git a/pkg/sql/metric_test.go b/pkg/sql/metric_test.go index c1c9fd1c1c09..7c010262d08d 100644 --- a/pkg/sql/metric_test.go +++ b/pkg/sql/metric_test.go @@ -35,6 +35,7 @@ type queryCounter struct { expectError bool txnBeginCount int64 selectCount int64 + selectExecutedCount int64 distSQLSelectCount int64 optCount int64 fallbackCount int64 @@ -43,6 +44,7 @@ type queryCounter struct { deleteCount int64 ddlCount int64 miscCount int64 + miscExecutedCount int64 failureCount int64 txnCommitCount int64 txnRollbackCount int64 @@ -69,10 +71,10 @@ func TestQueryCounts(t *testing.T) { var testcases = []queryCounter{ // The counts are deltas for each query. - {query: "SET OPTIMIZER = 'off'", miscCount: 1, fallbackCount: 1}, - {query: "SET DISTSQL = 'off'", miscCount: 1}, + {query: "SET OPTIMIZER = 'off'", miscCount: 1, miscExecutedCount: 1, fallbackCount: 1}, + {query: "SET DISTSQL = 'off'", miscCount: 1, miscExecutedCount: 1}, {query: "BEGIN; END", txnBeginCount: 1, txnCommitCount: 1}, - {query: "SELECT 1", selectCount: 1, txnCommitCount: 1}, + {query: "SELECT 1", selectCount: 1, selectExecutedCount: 1, txnCommitCount: 1}, {query: "CREATE DATABASE mt", ddlCount: 1}, {query: "CREATE TABLE mt.n (num INTEGER PRIMARY KEY)", ddlCount: 1}, {query: "INSERT INTO mt.n VALUES (3)", insertCount: 1}, @@ -86,22 +88,28 @@ func TestQueryCounts(t *testing.T) { {query: "UPDATE mt.n SET num = num + 1", updateCount: 1}, {query: "DELETE FROM mt.n", deleteCount: 1}, {query: "ALTER TABLE mt.n ADD COLUMN num2 INTEGER", ddlCount: 1}, - {query: "EXPLAIN SELECT * FROM mt.n", miscCount: 1}, + {query: "EXPLAIN SELECT * FROM mt.n", miscCount: 1, miscExecutedCount: 1}, { query: "BEGIN; UPDATE mt.n SET num = num + 1; END", txnBeginCount: 1, updateCount: 1, txnCommitCount: 1, }, - {query: "SELECT * FROM mt.n; SELECT * FROM mt.n; SELECT * FROM mt.n", selectCount: 3}, - {query: "SET DISTSQL = 'on'", miscCount: 1}, - {query: "SELECT * FROM mt.n", selectCount: 1, distSQLSelectCount: 1}, - {query: "SET DISTSQL = 'off'", miscCount: 1}, + { + query: "SELECT * FROM mt.n; SELECT * FROM mt.n; SELECT * FROM mt.n", + selectCount: 3, selectExecutedCount: 3, + }, + {query: "SET DISTSQL = 'on'", miscCount: 1, miscExecutedCount: 1}, + { + query: "SELECT * FROM mt.n", + selectCount: 1, selectExecutedCount: 1, distSQLSelectCount: 1, + }, + {query: "SET DISTSQL = 'off'", miscCount: 1, miscExecutedCount: 1}, {query: "DROP TABLE mt.n", ddlCount: 1}, - {query: "SET database = system", miscCount: 1}, - {query: "SET OPTIMIZER = 'on'", miscCount: 1}, - {query: "SELECT 3", selectCount: 1, optCount: 1}, + {query: "SET database = system", miscCount: 1, miscExecutedCount: 1}, + {query: "SET OPTIMIZER = 'on'", miscCount: 1, miscExecutedCount: 1}, + {query: "SELECT 3", selectCount: 1, selectExecutedCount: 1, optCount: 1}, {query: "CREATE TABLE mt.n (num INTEGER PRIMARY KEY)", ddlCount: 1, optCount: 1}, {query: "UPDATE mt.n SET num = num + 1", updateCount: 1, optCount: 1}, - {query: "SET OPTIMIZER = 'off'", miscCount: 1, fallbackCount: 1}, + {query: "SET OPTIMIZER = 'off'", miscCount: 1, miscExecutedCount: 1, fallbackCount: 1}, } accum := initializeQueryCounter(s) @@ -118,34 +126,40 @@ func TestQueryCounts(t *testing.T) { } var err error - if accum.txnBeginCount, err = checkCounterDelta(s, sql.MetaTxnBegin, accum.txnBeginCount, tc.txnBeginCount); err != nil { + if accum.txnBeginCount, err = checkCounterDelta(s, sql.MetaTxnBeginStarted, accum.txnBeginCount, tc.txnBeginCount); err != nil { t.Errorf("%q: %s", tc.query, err) } if accum.distSQLSelectCount, err = checkCounterDelta(s, sql.MetaDistSQLSelect, accum.distSQLSelectCount, tc.distSQLSelectCount); err != nil { t.Errorf("%q: %s", tc.query, err) } - if accum.txnRollbackCount, err = checkCounterDelta(s, sql.MetaTxnRollback, accum.txnRollbackCount, tc.txnRollbackCount); err != nil { + if accum.txnRollbackCount, err = checkCounterDelta(s, sql.MetaTxnRollbackStarted, accum.txnRollbackCount, tc.txnRollbackCount); err != nil { t.Errorf("%q: %s", tc.query, err) } if accum.txnAbortCount, err = checkCounterDelta(s, sql.MetaTxnAbort, accum.txnAbortCount, 0); err != nil { t.Errorf("%q: %s", tc.query, err) } - if accum.selectCount, err = checkCounterDelta(s, sql.MetaSelect, accum.selectCount, tc.selectCount); err != nil { + if accum.selectCount, err = checkCounterDelta(s, sql.MetaSelectStarted, accum.selectCount, tc.selectCount); err != nil { + t.Errorf("%q: %s", tc.query, err) + } + if accum.selectExecutedCount, err = checkCounterDelta(s, sql.MetaSelectExecuted, accum.selectExecutedCount, tc.selectExecutedCount); err != nil { + t.Errorf("%q: %s", tc.query, err) + } + if accum.updateCount, err = checkCounterDelta(s, sql.MetaUpdateStarted, accum.updateCount, tc.updateCount); err != nil { t.Errorf("%q: %s", tc.query, err) } - if accum.updateCount, err = checkCounterDelta(s, sql.MetaUpdate, accum.updateCount, tc.updateCount); err != nil { + if accum.insertCount, err = checkCounterDelta(s, sql.MetaInsertStarted, accum.insertCount, tc.insertCount); err != nil { t.Errorf("%q: %s", tc.query, err) } - if accum.insertCount, err = checkCounterDelta(s, sql.MetaInsert, accum.insertCount, tc.insertCount); err != nil { + if accum.deleteCount, err = checkCounterDelta(s, sql.MetaDeleteStarted, accum.deleteCount, tc.deleteCount); err != nil { t.Errorf("%q: %s", tc.query, err) } - if accum.deleteCount, err = checkCounterDelta(s, sql.MetaDelete, accum.deleteCount, tc.deleteCount); err != nil { + if accum.ddlCount, err = checkCounterDelta(s, sql.MetaDdlStarted, accum.ddlCount, tc.ddlCount); err != nil { t.Errorf("%q: %s", tc.query, err) } - if accum.ddlCount, err = checkCounterDelta(s, sql.MetaDdl, accum.ddlCount, tc.ddlCount); err != nil { + if accum.miscCount, err = checkCounterDelta(s, sql.MetaMiscStarted, accum.miscCount, tc.miscCount); err != nil { t.Errorf("%q: %s", tc.query, err) } - if accum.miscCount, err = checkCounterDelta(s, sql.MetaMisc, accum.miscCount, tc.miscCount); err != nil { + if accum.miscExecutedCount, err = checkCounterDelta(s, sql.MetaMiscExecuted, accum.miscExecutedCount, tc.miscExecutedCount); err != nil { t.Errorf("%q: %s", tc.query, err) } if accum.failureCount, err = checkCounterDelta(s, sql.MetaFailure, accum.failureCount, tc.failureCount); err != nil { @@ -216,16 +230,16 @@ func TestAbortCountConflictingWrites(t *testing.T) { if _, err := checkCounterDelta(s, sql.MetaTxnAbort, accum.txnAbortCount, 1); err != nil { t.Error(err) } - if _, err := checkCounterDelta(s, sql.MetaTxnBegin, accum.txnBeginCount, 1); err != nil { + if _, err := checkCounterDelta(s, sql.MetaTxnBeginStarted, accum.txnBeginCount, 1); err != nil { t.Error(err) } - if _, err := checkCounterDelta(s, sql.MetaTxnRollback, accum.txnRollbackCount, 0); err != nil { + if _, err := checkCounterDelta(s, sql.MetaTxnRollbackStarted, accum.txnRollbackCount, 0); err != nil { t.Error(err) } - if _, err := checkCounterDelta(s, sql.MetaTxnCommit, accum.txnCommitCount, 0); err != nil { + if _, err := checkCounterDelta(s, sql.MetaTxnCommitStarted, accum.txnCommitCount, 0); err != nil { t.Error(err) } - if _, err := checkCounterDelta(s, sql.MetaInsert, accum.insertCount, 1); err != nil { + if _, err := checkCounterDelta(s, sql.MetaInsertStarted, accum.insertCount, 1); err != nil { t.Error(err) } } @@ -252,10 +266,10 @@ func TestAbortCountErrorDuringTransaction(t *testing.T) { if _, err := checkCounterDelta(s, sql.MetaTxnAbort, accum.txnAbortCount, 1); err != nil { t.Error(err) } - if _, err := checkCounterDelta(s, sql.MetaTxnBegin, accum.txnBeginCount, 1); err != nil { + if _, err := checkCounterDelta(s, sql.MetaTxnBeginStarted, accum.txnBeginCount, 1); err != nil { t.Error(err) } - if _, err := checkCounterDelta(s, sql.MetaSelect, accum.selectCount, 1); err != nil { + if _, err := checkCounterDelta(s, sql.MetaSelectStarted, accum.selectCount, 1); err != nil { t.Error(err) } @@ -291,13 +305,13 @@ func TestSavepointMetrics(t *testing.T) { t.Fatal(err) } - if _, err := checkCounterDelta(s, sql.MetaRestartSavepoint, accum.restartSavepointCount, 1); err != nil { + if _, err := checkCounterDelta(s, sql.MetaRestartSavepointStarted, accum.restartSavepointCount, 1); err != nil { t.Error(err) } - if _, err := checkCounterDelta(s, sql.MetaRestartSavepoint, accum.releaseRestartSavepointCount, 1); err != nil { + if _, err := checkCounterDelta(s, sql.MetaRestartSavepointStarted, accum.releaseRestartSavepointCount, 1); err != nil { t.Error(err) } - if _, err := checkCounterDelta(s, sql.MetaRestartSavepoint, accum.rollbackToRestartSavepointCount, 1); err != nil { + if _, err := checkCounterDelta(s, sql.MetaRestartSavepointStarted, accum.rollbackToRestartSavepointCount, 1); err != nil { t.Error(err) } @@ -312,7 +326,7 @@ func TestSavepointMetrics(t *testing.T) { if err := txn.Rollback(); err != nil { t.Fatal(err) } - if _, err := checkCounterDelta(s, sql.MetaSavepoint, accum.savepointCount, 1); err != nil { + if _, err := checkCounterDelta(s, sql.MetaSavepointStarted, accum.savepointCount, 1); err != nil { t.Error(err) } @@ -330,7 +344,7 @@ func TestSavepointMetrics(t *testing.T) { if err := txn.Rollback(); err != nil { t.Fatal(err) } - if _, err := checkCounterDelta(s, sql.MetaRestartSavepoint, accum.restartSavepointCount, 2); err != nil { + if _, err := checkCounterDelta(s, sql.MetaRestartSavepointStarted, accum.restartSavepointCount, 2); err != nil { t.Error(err) } } diff --git a/pkg/sql/metric_util_test.go b/pkg/sql/metric_util_test.go index 558f13d872c7..9e0a1095a185 100644 --- a/pkg/sql/metric_util_test.go +++ b/pkg/sql/metric_util_test.go @@ -27,22 +27,24 @@ import ( // migrations that may have run DDL statements. func initializeQueryCounter(s serverutils.TestServerInterface) queryCounter { return queryCounter{ - txnBeginCount: s.MustGetSQLCounter(sql.MetaTxnBegin.Name), - selectCount: s.MustGetSQLCounter(sql.MetaSelect.Name), + txnBeginCount: s.MustGetSQLCounter(sql.MetaTxnBeginStarted.Name), + selectCount: s.MustGetSQLCounter(sql.MetaSelectStarted.Name), + selectExecutedCount: s.MustGetSQLCounter(sql.MetaSelectExecuted.Name), optCount: s.MustGetSQLCounter(sql.MetaSQLOpt.Name), distSQLSelectCount: s.MustGetSQLCounter(sql.MetaDistSQLSelect.Name), - updateCount: s.MustGetSQLCounter(sql.MetaUpdate.Name), - insertCount: s.MustGetSQLCounter(sql.MetaInsert.Name), - deleteCount: s.MustGetSQLCounter(sql.MetaDelete.Name), - ddlCount: s.MustGetSQLCounter(sql.MetaDdl.Name), - miscCount: s.MustGetSQLCounter(sql.MetaMisc.Name), - txnCommitCount: s.MustGetSQLCounter(sql.MetaTxnCommit.Name), - txnRollbackCount: s.MustGetSQLCounter(sql.MetaTxnRollback.Name), + updateCount: s.MustGetSQLCounter(sql.MetaUpdateStarted.Name), + insertCount: s.MustGetSQLCounter(sql.MetaInsertStarted.Name), + deleteCount: s.MustGetSQLCounter(sql.MetaDeleteStarted.Name), + ddlCount: s.MustGetSQLCounter(sql.MetaDdlStarted.Name), + miscCount: s.MustGetSQLCounter(sql.MetaMiscStarted.Name), + miscExecutedCount: s.MustGetSQLCounter(sql.MetaMiscExecuted.Name), + txnCommitCount: s.MustGetSQLCounter(sql.MetaTxnCommitStarted.Name), + txnRollbackCount: s.MustGetSQLCounter(sql.MetaTxnRollbackStarted.Name), txnAbortCount: s.MustGetSQLCounter(sql.MetaTxnAbort.Name), - savepointCount: s.MustGetSQLCounter(sql.MetaSavepoint.Name), - restartSavepointCount: s.MustGetSQLCounter(sql.MetaRestartSavepoint.Name), - releaseRestartSavepointCount: s.MustGetSQLCounter(sql.MetaReleaseRestartSavepoint.Name), - rollbackToRestartSavepointCount: s.MustGetSQLCounter(sql.MetaRollbackToRestartSavepoint.Name), + savepointCount: s.MustGetSQLCounter(sql.MetaSavepointStarted.Name), + restartSavepointCount: s.MustGetSQLCounter(sql.MetaRestartSavepointStarted.Name), + releaseRestartSavepointCount: s.MustGetSQLCounter(sql.MetaReleaseRestartSavepointStarted.Name), + rollbackToRestartSavepointCount: s.MustGetSQLCounter(sql.MetaRollbackToRestartSavepointStarted.Name), } } diff --git a/pkg/sql/pgwire/server.go b/pkg/sql/pgwire/server.go index 8051b97abad7..737d064c8439 100644 --- a/pkg/sql/pgwire/server.go +++ b/pkg/sql/pgwire/server.go @@ -284,9 +284,11 @@ func (s *Server) IsDraining() bool { func (s *Server) Metrics() (res []interface{}) { return []interface{}{ &s.metrics, - &s.SQLServer.Metrics.StatementCounters, + &s.SQLServer.Metrics.StartedStatementCounters, + &s.SQLServer.Metrics.ExecutedStatementCounters, &s.SQLServer.Metrics.EngineMetrics, - &s.SQLServer.InternalMetrics.StatementCounters, + &s.SQLServer.InternalMetrics.StartedStatementCounters, + &s.SQLServer.InternalMetrics.ExecutedStatementCounters, &s.SQLServer.InternalMetrics.EngineMetrics, } } diff --git a/pkg/sql/txn_restart_test.go b/pkg/sql/txn_restart_test.go index 436e9d1b2c18..1e65526a6be8 100644 --- a/pkg/sql/txn_restart_test.go +++ b/pkg/sql/txn_restart_test.go @@ -818,7 +818,7 @@ CREATE TABLE t.test (k INT PRIMARY KEY, v TEXT); t.Fatal(err) } - commitCount := s.MustGetSQLCounter(sql.MetaTxnCommit.Name) + commitCount := s.MustGetSQLCounter(sql.MetaTxnCommitStarted.Name) // This is the magic. Run the txn closure until all the retries are exhausted. retryExec(t, sqlDB, rs, func(tx *gosql.Tx) bool { return runTestTxn(t, tc.magicVals, tc.expectedErr, sqlDB, tx, sentinelInsert) @@ -845,7 +845,7 @@ CREATE TABLE t.test (k INT PRIMARY KEY, v TEXT); // Check that the commit counter was incremented. It could have been // incremented by more than 1 because of the transactions we use to force // aborts, plus who knows what else the server is doing in the background. - if err := checkCounterGE(s, sql.MetaTxnCommit, commitCount+1); err != nil { + if err := checkCounterGE(s, sql.MetaTxnCommitStarted, commitCount+1); err != nil { t.Error(err) } // Clean up the table for the next test iteration. diff --git a/pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/overview.tsx b/pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/overview.tsx index 87f435c81c6f..65f3265f12dc 100644 --- a/pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/overview.tsx +++ b/pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/overview.tsx @@ -30,7 +30,7 @@ export default function (props: GraphDashboardProps) { sources={nodeSources} tooltip={ `A ten-second moving average of the # of SELECT, INSERT, UPDATE, and DELETE statements - started per second ${tooltipSelection}.` + successfully executed per second ${tooltipSelection}.` } > diff --git a/pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/sql.tsx b/pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/sql.tsx index b566bada473e..90c89acd628b 100644 --- a/pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/sql.tsx +++ b/pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/sql.tsx @@ -52,7 +52,7 @@ export default function (props: GraphDashboardProps) { sources={nodeSources} tooltip={ `A ten-second moving average of the # of SELECT, INSERT, UPDATE, and DELETE statements - started per second ${tooltipSelection}.` + successfully executed per second ${tooltipSelection}.` } >