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

Jump to bottom

sync: TestPool failure on windows-amd64-2016 #20198

Closed
josharian opened this issue May 1, 2017 · 16 comments
Closed

sync: TestPool failure on windows-amd64-2016 #20198

josharian opened this issue May 1, 2017 · 16 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.9

Comments

@josharian
Copy link
Contributor

Noticed on build.golang.org.

https://build.golang.org/log/a3f357b132ef8cb703e42085a50c8c4499e64301

##### sync -cpu=10
--- FAIL: TestPool (0.00s)
	pool_test.go:29: got "b"; want a
FAIL
FAIL	sync	0.412s

First showed up after CL 42090, although that seems unlikely to be the cause.

cc @valyala @dvyukov
@josharian josharian added Builders x/build issues (builders, bots, dashboards) Testing An issue that has been verified to require only test changes, not just a test failure. labels May 1, 2017
@josharian josharian added this to the Go1.9 milestone May 1, 2017
@josharian josharian removed the Builders x/build issues (builders, bots, dashboards) label May 1, 2017
@josharian
Copy link
Contributor Author

@dlespiau

@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 1, 2017
@bradfitz
Copy link
Contributor

bradfitz commented May 1, 2017

/cc @aclements for log history greppin'.

@dlespiau
Copy link
Contributor

dlespiau commented May 1, 2017

It seems unlikely to be caused by CL 42090 indeed. That (SSE) instruction isn't generated by the compiler and not present in any of the .s files in the go project (well, apart from the testdata file checking its correctness).

@dvyukov
Copy link
Member

dvyukov commented May 2, 2017

Perhaps the goroutine was rescheduled to another P (due to preemption?).
runtime.GOMAXPROCS(1) should with against that.

@bradfitz
Copy link
Contributor

bradfitz commented May 4, 2017

Also openbsd-386:
https://build.golang.org/log/71a13d246b73925bd56923c0a706a95cbaf74e74
and others

@bradfitz
Copy link
Contributor

bradfitz commented May 4, 2017

@aclements, this started happening pretty frequently. Do you know what changed?

@gopherbot
Copy link

CL https://golang.org/cl/42770 mentions this issue.

@josharian
Copy link
Contributor Author

The only recent sync.Pool change I can think of is CL 40913.

@bradfitz
Copy link
Contributor

bradfitz commented May 4, 2017

That one seems safe.

@josharian
Copy link
Contributor Author

Oh, also CL 40918.

@aclements
Copy link
Member

I'm only seeing three failures ever. Did I grep this wrong?

$ greplogs -dashboard -E "FAIL: TestPool" -md -l
2017-04-29T04:14:36-b225396/linux-386
2017-05-02T20:03:30-9f6fde3/nacl-amd64p32
2017-05-04T21:55:36-8d63408/openbsd-386-60

@bradfitz
Copy link
Contributor

bradfitz commented May 5, 2017

@aclements, I've seen a number of trybot failures too, and those three URLs are all of the form used by non-trybot failures.

@aclements
Copy link
Member

Okay. If it's failing fairly regularly on the trybots, can we bisect this?

@dvyukov
Copy link
Member

dvyukov commented May 5, 2017

Note that the test is formally broken/incorrect. So any innocent change can suddenly cause increased failure rate it in. Does cl/42770 help? We could also use procPin/procUnpin around the test, that should reflect the intention of the test -- within same P reuse order is LIFO.

@ianlancetaylor
Copy link
Contributor

Just saw this on nacl-386: https://storage.googleapis.com/go-build-log/b9150e2c/nacl-386_6f3075c2.log

@gopherbot
Copy link

CL https://golang.org/cl/44310 mentions this issue.

@golang golang locked and limited conversation to collaborators Jun 28, 2018
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. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
7 participants
@bradfitz @dlespiau @josharian @dvyukov @aclements @ianlancetaylor @gopherbot