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

cmd/go: go test with nocoverageredesign uses huge amount temp files #66428

Closed
muhlemmer opened this issue Mar 20, 2024 · 5 comments
Closed

cmd/go: go test with nocoverageredesign uses huge amount temp files #66428

muhlemmer opened this issue Mar 20, 2024 · 5 comments
Assignees
@thanm
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@muhlemmer
Copy link
Contributor

Go version

go version go1.22.1 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/tim/.cache/go-build'
GOENV='/home/tim/.config/go/env'
GOEXE=''
GOEXPERIMENT='nocoverageredesign'
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/tim/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/tim/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/tim/Repositories/goroot'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/tim/Repositories/goroot/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.1'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3522648992=/tmp/go-build -gno-record-gcc-switches'

What did you do?

While waiting on the backport of this issue #65653, we decided to apply the work-around of setting GOEXPERIMENT=nocoverageredesign. This resulted in the go test command to consume huge amount storage under /tmp.

Steps to reproduce, (apologies for including our repo, but I was not able to get significant results with small examples):

  1. go env -w GOEXPERIMENT=nocoverageredesign
  2. git clone https://github.com/zitadel/zitadel.git
  3. cd zitadel
  4. make core_api core_assets (installs protobuf, grpc and generates some stuff)
  5. go test -race -coverprofile=profile.cov -coverpkg=./internal/...,./cmd/... ./...

What did you see happen?

As my /tmp is a tmpfs, it uses a restricted amount of space, 7.5G to be exact. Some of that is used by other apps, but usualy never more than 1G. While the test runs I'm getting errors like:

# github.com/zitadel/zitadel/internal/api [github.com/zitadel/zitadel/internal/idp/providers/oidc.test]
compile: writing output: write $WORK/b4043/_pkg_.a: no space left on device
github.com/zitadel/zitadel/internal/api/grpc/auth: write /tmp/go-build3854946245/b4046/importcfg: no space left on device
github.com/zitadel/zitadel/internal/api/idp: write /tmp/go-build3854946245/b4056/importcfg: no space left on device
github.com/zitadel/zitadel/internal/api/saml: write /tmp/go-build3854946245/b4057/importcfg: no space left on device
github.com/zitadel/zitadel/internal/api/grpc/user/schema/v3alpha: write /tmp/go-build3854946245/b4098/importcfg: no space left on device
github.com/zitadel/zitadel/internal/logstore/emitters/access: write /tmp/go-build3854946245/b4101/importcfg: no space left on device
github.com/zitadel/zitadel/internal/logstore/emitters/execution: write /tmp/go-build3854946245/b4102/importcfg: no space left on device

Further investigation shows, by running du -h /tmp/go-build* in a loop while running the test:

  1. running Go 1.22.x with GOEXPERIMENT=nocoverageredesign : consumed space comes close to 7GB, at which point /tmp is full
  2. running Go 1.22.x without GOEXPERIMENT: consumed space stays under 2GB, but confronts us with cmd/go: go 1.22.0: go test throws errors when processing folders not listed in coverpkg argument #65653
  3. running Go 1.21.x without GOEXPERIMENT: consumed space stays under 2GB.

I would also like to note that I first encountered this locally on my system with the tmpfs setup, which might be considered custom. However, today I was confronted with a colleague trying to upgrade Go in our pipeline and the same bug essentially trashed the Github runner. This convinced me to raise the issue after all. (CC @adlerhurst)

What did you expect to see?

When disabling an experimental feature, the go test command should behave as the version before the feature was added, not introduce another type of bug.
@dr2chase
Copy link
Contributor

@thanm

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 21, 2024
@thanm
Copy link
Contributor

thanm commented Mar 21, 2024

Thanks for the report. I will take a look.

@thanm thanm self-assigned this Mar 21, 2024
@thanm
Copy link
Contributor

thanm commented Mar 21, 2024

Very much appreciate your filing this bug, it looks like this has sailed right under our testing radar.

A little background:

Prior to any of the coverage redesign work (and today on tip when users turn off new coverage with GOEXPERIMENT=nocoverageredesign), when a user runs a command like

   go test -coverpgkg=<PATTERN1> <PATTERN2>

the Go command will take all of the packages matching <PATTERN1> and force-import each one into the _testmain.go file for each package in <PATTERN2>. This can include some rather weird scenarios, e.g. the testmain binary being built to run package A's tests would import package B, even if A is actually a dependency of B.

It looks as though the bug has to do with package build action lookup and/or caching code in the Go command; for the force-imports required as described above, instead of reusing results it winds up building multiple copies of the same package.

For the reproducer you posted, for example, we wind up building 14 separate copies of the package "github.com/zitadel/zitadel/pkg/grpc/management", ugh.

Not yet sure what triggers this-- my attempts to get it to happen for toy programs (e.g. https://go.dev/play/p/9AswxU2WCIj?v=gotip). I'll spend a little more time on it later today.

@thanm
Copy link
Contributor

thanm commented Mar 21, 2024

OK, after looking a bit more closely, it appears that this is not a new issue per se.

Attached is a shell script that will generate a dummy set of 20 packages, p1 through p20. p2 depends on p1; p3 depends on p2 and p1; p4 depends on p3/p2/p1 and so on. Play link at https://go.dev/play/p/B-iWSKqjX1b .

shellscript.txt

If you run

GOEXPERIMENT=nocoverageredesign go test -coverpkg=./... ./...

on tip with this set of packages this will reproduce the problem. You can't see it without inspecting the "go test -x" output, but essentially what happens is that for each package pk testmain, we create new instances of each of the dependent covered packages that pk needs, so as you might expect there is a big blowup in testing time and amount of disk space required.

The code that's causing the duplication is here:

https://go.googlesource.com/go/+/45b641ce15159e29fa4494b837493042d1e10384/src/cmd/go/internal/load/test.go#449

which has been around in one form or another since Go 1.11 in fact. If I do a "go test -x -coverpkg=./... ./..." using Go 1.19 without the new coverage I can see just the same sort of duplication.

So in essence this is not really a regression or a new issue, it's just another one of the many problems that we had with the way old-style coverage worked (due once again to the forced imports).

At this point it seems unlikely that we would want to attempt a fix given that this is something that has been hanging around for a while.

What I am going suggest is that instead we ask users to wait for the next Go 1.22 minor release. The fix for https://go.dev/issue/65653 has already been cherry-picked onto the 1.22 release branch, and in theory we should have a minor release going out next week.

Let me know if this sounds ok.

@muhlemmer
Copy link
Contributor Author

Makes sense and works for us. Thanks for checking this out!

@muhlemmer muhlemmer closed this as not planned Won't fix, can't repro, duplicate, stale Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thanm thanm

Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dr2chase @muhlemmer @thanm