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: TestStmtCloseDeps failure on windows-amd64-longtest #49958

Closed
bcmills opened this issue Dec 3, 2021 · 6 comments
Closed

database/sql: TestStmtCloseDeps failure on windows-amd64-longtest #49958

bcmills opened this issue Dec 3, 2021 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.18

Comments

@bcmills
Copy link
Contributor

bcmills commented Dec 3, 2021 

--- FAIL: TestStmtCloseDeps (5.11s)
    sql_test.go:2502: len(stmt.css) = 59; want <= 50
FAIL
FAIL	database/sql	6.414s

greplogs --dashboard -md -l -e 'FAIL: TestStmtCloseDeps ' --since=2020-01-01

2021-11-27T19:49:32-a142d65/windows-amd64-longtest

This is the regression test for #5323. Only one failure has been seen in the logs, but since the test is skipped it short mode the sample size is relatively smaller than for most tests; however, there have been fairly recent changes in database/sql (CC @kardianos, @ianlancetaylor).
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 3, 2021
@bcmills bcmills added this to the Backlog milestone Dec 3, 2021
@kardianos
Copy link
Contributor

Thanks. I'll try to duplicate ASAP and diagnose. I agree that fundamental changes were recently made.

@kardianos
Copy link
Contributor

@bcmills I was unable to reproduce locally to date. When looking at the test itself, it contains a time.Sleep call that when I reduce the amount of time, it increase the number of open statements and fails the test. As such, I somewhat assume that perhaps this machine was loaded and starved the process of CPU for a bit and it failed to complete all the steps.

I could (a) ignore this, or (b) try to modify the test to not include this sleep.

@bcmills
Copy link
Contributor Author

bcmills commented Dec 7, 2021

Given that users' development configurations and CI systems are likely even more variable than the Go project's builders, it seems like a good idea to modify the test to avoid arbitrary sleeps.

@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Dec 7, 2021
@kardianos
Copy link
Contributor

database/sql tests are moving that direction; I'll see what I can do to remove the dependency on sleeps.

@bcmills
Copy link
Contributor Author

bcmills commented Jan 12, 2022

greplogs --dashboard -md -l -e 'FAIL: TestStmtCloseDeps ' --since=2021-12-04

2022-01-11T21:51:51-ad7eae2/windows-amd64-longtest
2021-12-16T22:39:00-b1a53ec/windows-amd64-longtest
2021-12-12T14:36:21-8692bac/windows-amd64-longtest
2021-12-06T14:58:33-f8a8a73/windows-amd64-longtest

This is looking like a regression in Go 1.18 — possibly related to the runtime's pacing changes? (CC @mknyszek)

@bcmills bcmills modified the milestones: Backlog, Go1.18 Jan 12, 2022
@bcmills bcmills self-assigned this Jan 13, 2022
@gopherbot
Copy link

Change https://golang.org/cl/378395 mentions this issue: database/sql: consolidate test polling loops

jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
@jproberts
database/sql: consolidate test polling loops
669ba02 
Also eliminate some arbitrary deadlines and sleeps.

Fixes golang#49958

Change-Id: I999b39a896e430e3bb93aa8b8c9444f28bbaa9d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/378395
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
@rsc rsc unassigned bcmills Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
3 participants
@kardianos @bcmills @gopherbot