Skip to content

Commit

Permalink
pkg/sql/sqlstats: de-flake TestTransactionInsightsFailOnCommit
Browse files Browse the repository at this point in the history
Fixes: #117061

TestTransactionInsightsFailOnCommit runs two transactions, one
of which is expected to fail on commit due to an isolation error
as it interacts with the other transaction.

We query crdb_internal.node_txn_execution_insights, which queries
the in-memory insights data, to check that a txn insight was generated
for the failed txn with a `FailedExecution` insight.

However, when querying the crdb_internal table, the query is too broad.
In the test, the successful transaction can still possibly generate a
`SlowExecution` insight. In this case, when looking for the
`FailedExecution` insight, we were instead getting an insight from the
successful transaction.

The fix is to narrow the SELECT criteria to ensure we're selecting the
correct insight.

The fix also expands the failed assertion logging.

Release note: none
  • Loading branch information
abarganier committed Mar 28, 2024
1 parent f8dc5a0 commit eb77666
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions pkg/sql/sqlstats/insights/integration/insights_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,12 +490,16 @@ SELECT query,
COALESCE(last_error_code, '') last_error_code,
COALESCE(last_error_redactable, '') last_error
FROM crdb_internal.node_txn_execution_insights
WHERE app_name = $1`, appName)
WHERE app_name = $1
AND query = 'SELECT * FROM myusers WHERE city = _ ; UPDATE myusers SET name = _ WHERE city = _'`, appName)

return row.Scan(&query, &problems, &status, &errorCode, &errorMsg)
})

require.Equal(t, "SELECT * FROM myusers WHERE city = _ ; UPDATE myusers SET name = _ WHERE city = _", query)
require.Equalf(t,
"SELECT * FROM myusers WHERE city = _ ; UPDATE myusers SET name = _ WHERE city = _", query,
"unexpected txn insight found - query: %s, problems: %s, status: %s, errCode: %s, errMsg: %s",
query, problems, status, errorCode, errorMsg)
expectedProblem := "{FailedExecution}"
replacedSlowProblems := problems
if problems != expectedProblem {
Expand Down

0 comments on commit eb77666

Please sign in to comment.