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

database/sql: TestTxContextWaitNoDiscard failure due to race between timeouts #53222

Closed
bcmills opened this issue Jun 3, 2022 · 2 comments
Closed

database/sql: TestTxContextWaitNoDiscard failure due to race between timeouts #53222

bcmills opened this issue Jun 3, 2022 · 2 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Jun 3, 2022 

--- FAIL: TestTxContextWaitNoDiscard (1.27s)
    sql_test.go:501: expected QueryContext to error with context deadline exceeded but returned <nil>
FAIL
FAIL	database/sql	7.159s

greplogs -l -e 'FAIL: TestTxContextWaitNoDiscard'
2022-06-03T00:13:09-7846e25/freebsd-arm-paulzhol

This test employs two arbitrary timeouts: the transaction times out after 15 ms, and the query completes after 1 s.

However, Context timeouts are asynchronous and propagate on best-effort. If the timeout propagation happens to be delayed by one second (as can easily happen on a slow and/or overloaded builder), then the query can unexpectedly complete.

Probably the best solution is to change the query to wait for an unbounded time instead of just one second.

(attn @bradfitz @kardianos; CC @kevinburke per https://dev.golang.org/owners)
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jun 3, 2022
@bcmills bcmills added this to the Backlog milestone Jun 3, 2022
@bcmills
Copy link
Contributor Author

bcmills commented Jun 3, 2022

(CC @paulzhol, because this builder-independent issue was discovered on his builder)

@gopherbot
Copy link

Change https://go.dev/cl/416655 mentions this issue: database/sql: make TestTxContextWaitNoDiscard test more robust

jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@dmgk @jproberts
database/sql: make TestTxContextWaitNoDiscard test more robust
48e73c6 
Similar to CL 385934, rely on waiter trigger instead of the WAIT query
prefix and factor out the common test code.

Fixes golang#53222

Change-Id: I46efc85ca102b350bb4dbe8e514921e016870ffb
Reviewed-on: https://go-review.googlesource.com/c/go/+/416655
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Goutnik <dgoutnik@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
@golang golang locked and limited conversation to collaborators Jul 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
2 participants
@bcmills @gopherbot