Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

release-19.1: sql: fix the auto-retry counter in stats + log it in statement/audit logs #38035

Merged
merged 3 commits into from Jun 19, 2019
Merged

release-19.1: sql: fix the auto-retry counter in stats + log it in statement/audit logs #38035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7fdc756
sql: fix the retry counter in the statement statistics
knz Jun 5, 2019
75c26e9
sql: deflake TestLogic//crdb_internal/max_retry_counter
knz Jun 6, 2019
3f3ed09
sql: logic_test/crdb_internal: add missing comments
knz Jun 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions pkg/sql/conn_executor.go
View file
Open in desktop
Expand Up @@ -1032,8 +1032,6 @@ func (ex *connExecutor) resetExtraTxnState(

ex.extraTxnState.tables.databaseCache = dbCacheHolder.getDatabaseCache()

ex.extraTxnState.autoRetryCounter = 0

// Close all portals.
for name, p := range ex.extraTxnState.prepStmtsNamespace.portals {
p.decRef(ctx)
Expand Down Expand Up @@ -1966,6 +1964,7 @@ func (ex *connExecutor) txnStateTransitionsApplyWrapper(
switch advInfo.txnEvent {
case noEvent:
case txnStart:
ex.extraTxnState.autoRetryCounter = 0
case txnCommit:
if res.Err() != nil {
err := errorutil.UnexpectedWithIssueErrorf(
Expand Down
41 changes: 41 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/crdb_internal
View file
Open in desktop
Expand Up @@ -375,3 +375,44 @@ query T
SELECT crdb_internal.pretty_key(e'\\xa82a00918ed9':::BYTES, (-5096189069466142898):::INT8);
----
/Table/32/???/9/6/81

subtest max_retry_counter
# Verify that the max_retry counter in statement stats actually increases with retries.

statement ok
SET application_name = 'test_max_retry'

# Make the statement retry, to ensure max_retries increases to
# become different from 0.
# The delay is chosen identically to the value in the `txn`
# logic tests introduced in #16719. It is long enough
# to ensure max_retries increases, even under stress,
# but short enough to be negligible compared to the
# overall test suite.
statement OK
SELECT crdb_internal.force_retry('50ms'::INTERVAL)

statement OK
RESET application_name

# Note: in the following test, three rows of output are expected:
# - one for the SELECT statements that failed with a retry error,
# - one for the final SELECT retry attempt that succeeded without an error,
# - one for the RESET statement.
#
# We expect the first two entries to have max_retries > 0 because
# auto-retries are expected by the server.
# We also expect the RESET statement to have max_retries = 0, because
# RESET never retries. This tests that the retry counter is properly
# reset to 0 between statements - a naive implementation could make
# the counter increase forever, even between statements.
#
query TBB
SELECT key, (max_retries > 0), flags LIKE '!%' AS f
FROM crdb_internal.node_statement_statistics
WHERE application_name = 'test_max_retry'
ORDER BY key, f
----
SELECT crdb_internal.force_retry(_) true false
SELECT crdb_internal.force_retry(_) true true
SET application_name = DEFAULT false false