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.

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

sql: TestTxnContentionEventsTable failed #102660

Closed
cockroach-teamcity opened this issue Apr 29, 2023 · 1 comment
Closed

sql: TestTxnContentionEventsTable failed #102660

cockroach-teamcity opened this issue Apr 29, 2023 · 1 comment
Assignees
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. T-sql-queries SQL Queries Team
Milestone

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Apr 29, 2023

sql.TestTxnContentionEventsTable failed with artifacts on master @ 1ee30a1fe7e56814a37c22457078c3a6bd0ede27:

=== RUN   TestTxnContentionEventsTable
    test_log_scope.go:161: test logs captured to: /artifacts/tmp/_tmp/004ac2cdb3e4dcce971cd33db6280729/logTestTxnContentionEventsTable369855696
    test_log_scope.go:79: use -show-logs to present logs inline
    crdb_internal_test.go:1069: condition failed to evaluate within 5s: transaction_contention_events did not return any rows
    panic.go:522: -- test log scope end --
test logs left over in: /artifacts/tmp/_tmp/004ac2cdb3e4dcce971cd33db6280729/logTestTxnContentionEventsTable369855696
--- FAIL: TestTxnContentionEventsTable (8.38s)
Help

See also: How To Investigate a Go Test Failure (internal)

/cc @cockroachdb/sql-queries

This test on roachdash | Improve this report!

Jira issue: CRDB-27575

@cockroach-teamcity cockroach-teamcity added branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. labels Apr 29, 2023
@cockroach-teamcity cockroach-teamcity added this to the 23.1 milestone Apr 29, 2023
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Apr 29, 2023
gtr added a commit to gtr/cockroach that referenced this issue May 15, 2023
gtr added a commit that referenced this issue May 16, 2023
craig bot pushed a commit that referenced this issue May 16, 2023
102808: kv: eliminate write-too-old deferral mechanism r=nvanbenschoten a=nvanbenschoten

KV half of #102751.

This commit eliminates the write-too-old deferral mechanism, where blind-write BatchRequests that hit a WriteTooOld error would successfully write intents and then return a Transaction proto with the WriteTooOld flag to the client. The client would then immediately refresh to remove this flag. However, in the intermediate period, the written intents would act as locks to ensure that if the refresh succeeded, the writer would have exclusive access to the previously written keys and would not hit a WriteTooOld error on its next attempt.

The rationale for the removal of this mechanism is outlined in #102751. At a high-level, the mechanism is complex, error-prone, and sufficiently unnecessary today due to unreplicated locks and server-side refreshes. It also interacts poorly with weak isolation levels, which further motivates its removal.

Cases where the write-too-old deferral mechanism is still hypothetically useful are difficult to construct, especially from SQL's limited use of KV. They require the following conditions to all hold:

1. a blind-writing BatchRequest (containing Put or Delete, but not ConditionalPut)
2. a BatchRequest without the CanForwardReadTimestamp flag (needs client-side refresh)
3. a write-write conflict that will not cause a refresh to fail

These requirement are almost always contradictory. A write-write conflict implies a failed refresh if the refresher has already read the conflicting key. So the cases where this mechanism help are limited to those where the writer has not already read the conflicting key. However, SQL rarely issues blind-write KV requests keys that it has not already read. The cases where this might come up are fast-path DELETE statements that issue DeleteRequest (not DeleteRangeRequest) and fast-path UPSERT statements that write all columns in a table. If either of these are heavily contended and take place in multi-statement transactions that previously read, this mechanism could help. However, I suspect that these scenarios are very uncommon. If customers do see them, they can avoid problems by using SELECT FOR UPDATE earlier in the transaction or by using Read Committed (which effectively resets the CanForwardReadTimestamp flag on each statement boundary).

The commit does not yet remove the Transaction.WriteTooOld field, as this must remain until compatibility with v23.1 nodes is no longer a concern.

Release note: None

103353: sql: skip flaky TestTxnContentionEventsTable r=gtr a=gtr

Informs #102660.

Release note: None

103354: admission: fix comment in admission.go r=bananabrick a=bananabrick

Epic: none
Release note: None

103404: concurrency: fix bug in lockStateInfo r=nvanbenschoten a=arulajmani

All waiting readers are considered to be actively waiting at a lock;
there's no concept of inactive waiting readers in the lock table.
Previously, when converting a lockState into a roachpb.LockStateInfo,
we would erroneously denote any waiting readers as inactive. This
patch fixes this and adds a test for it.

This patch also fixes how reservations are designated as active or
inactive. Previously, reservations would be marked as active waiters.
This isn't true -- reservation holders do not actively wait at a lock
they hold reservations for. They only wait at other locks or proceed
to evaluation. Now, we mark reservations as inactive waiters as well.
The diff in existing tests is a result of this change.

Epic: none

Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: gtr <[email protected]>
Co-authored-by: Arjun Nair <[email protected]>
Co-authored-by: Arul Ajmani <[email protected]>
gtr added a commit that referenced this issue Jun 1, 2023
craig bot pushed a commit that referenced this issue Jun 1, 2023
104206: sql: unskip TestTxnContentionEventsTable r=gtr a=gtr

Informs: #102660.

Release note: None

104208: indexrec: improve recommendations for queries with inequalities r=qiyanghe1998 a=qiyanghe1998

Fixes #102206

As for the tests, since !=, IS DISTINT FROM and IS NOT are categorized as range
index, so I copied similar tests with "R": R, EQ + R, J + R, EQ + J + R.

Release note: None

Epic: None

Co-authored-by: gtr <[email protected]>
Co-authored-by: qiyanghe1998 <[email protected]>
@gtr
Copy link
Contributor

gtr commented Jun 6, 2023

Closing as #104206 has merged and no failures have occurred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. T-sql-queries SQL Queries Team
Projects
None yet
Development

No branches or pull requests

2 participants