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

runtime: linux-arm builder intermittently failing TestPhysicalMemoryUtilization #32010

Closed
josharian opened this issue May 13, 2019 · 7 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@josharian
Copy link
Contributor

Sample from https://build.golang.org/log/964dcd623fe5a0545b1b96bf2b5368b67b17accc:

--- FAIL: TestPhysicalMemoryUtilization (0.15s)
    malloc_test.go:175: expected "OK\n", but got "exceeded physical memory overuse threshold of 10%: 11.42%\n(alloc: 378579448, sys: 507150336, rel: 85319680, objs: 240)\n"
FAIL
FAIL	runtime	73.024s

cc @aclements @mknyszek

Tentatively marking release-blocker, since we want the builders to be consistently green.

@josharian josharian added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels May 13, 2019
@josharian josharian added this to the Go1.13 milestone May 13, 2019
@mknyszek
Copy link
Contributor

Thanks for pointing this out and creating an issue. I've seen this before and it should be resolved sooner rather than later.

The test likely needs to be tweaked or revamped because of the new scavenger, or maybe if #32012 is fixed it won't really be relevant anymore.

I'll update this issue when I understand better what to do about this test.

@mknyszek mknyszek self-assigned this May 13, 2019
@mknyszek
Copy link
Contributor

OK so after thinking about this for a bit, and looking over the failures (which are all just failures due to rising slightly above the threshold), I think we should just raise the threshold.

The test is still useful, but the scavenging policy now is to retain a 10% overhead of RSS over the heap goal, which means that picking 10% as the test's threshold is probably not reasonable, especially since the scavenger isn't guaranteed to finish by the time the test actually checks what the utilization is. The test is still useful though, to ensure the scavenger is actually doing its job, but there's no guarantee anymore that it'll be under 10%. We could sleep to ensure the scavenger runs for long enough with a worst-case pace to ensure we get close to or below the threshold, but adding more time to all.bash seems like the wrong way to go.

Instead, increasing the threshold just accounts for any variability in the scavenger being able to complete its goal. It should be extremely likely for the test's utilization to be under 15%, especially with the fix to #32012, so I think we should just set it to that.

Thoughts?

@gopherbot
Copy link

Change https://golang.org/cl/177237 mentions this issue: runtime: increase physical memory utilization threshold in test

@laboger
Copy link
Contributor

laboger commented May 17, 2019

This problem has been happening intermittently on linux/ppc64 and linux/ppc64le.

@bradfitz
Copy link
Contributor

This now happens on linux-amd64-longtest too:
https://build.golang.org/log/13c1c2b621db5aa94a4aea1a6c88eabe48785e22

@shakeel
Copy link

shakeel commented May 20, 2019

Fails consistently on my local Linux workstation --- FAIL: TestPhysicalMemoryUtilization (0.01s) malloc_test.go:175: expected "OK\n", but got "exceeded physical memory overuse threshold of 10%: 18.15%\n(alloc: 378900688, sys: 536248320, rel: 88571904, objs: 240)\n" FAIL

@mknyszek
Copy link
Contributor

Yeah I can reproduce in linux/amd64 also. I've overhauled the test now and it passes very consistently on linux/amd64 and linux/arm. Waiting on review now for https://golang.org/cl/177237.

@golang golang locked and limited conversation to collaborators May 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

6 participants