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

x/tools/go/packages: test timeout on freebsd-amd64-race builder #34621

Closed
bcmills opened this issue Sep 30, 2019 · 10 comments
Closed

x/tools/go/packages: test timeout on freebsd-amd64-race builder #34621

bcmills opened this issue Sep 30, 2019 · 10 comments
Labels
Builders x/build issues (builders, bots, dashboards) 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. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@bcmills
Copy link
Contributor

bcmills commented Sep 30, 2019

https://build.golang.org/log/10a77e60ed1952a8bbd937801dfd04d3b8aab6d1 shows a test timeout on the freebsd-amd64-race builder.

It may be that the test was deadlocked, or it may be that the builder just needs a longer scale factor for running the test.

It looks like the test got stuck running a subprocess, so if the problem is a deadlock, it might help to have the go/packages tests terminate subprocesses when the test runs out of time so that we can get goroutine dumps. (The tests for the go command do something similar here.)

CC @ianthehat @matloob @heschik @dmitshur @toothrot @bradfitz
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 30, 2019
@gopherbot gopherbot added this to the Unreleased milestone Sep 30, 2019
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 30, 2019
@bcmills bcmills added Builders x/build issues (builders, bots, dashboards) and removed Tools This label describes issues relating to any tools in the x/tools repository. labels Sep 30, 2019
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 30, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/204200 mentions this issue: go/packages: add a timeout for TestLoadImportsC

gopherbot pushed a commit to golang/tools that referenced this issue Oct 31, 2019
@matloob
go/packages: add a timeout for TestLoadImportsC
d9fd88a 
This will help debug golang/go#34621 by threading a context that's
canceled when the test times out. This will kill a go command that's
running when the test times out (the go command is already run
using CommandContext with the config's context).

Updates golang/go#34621

Change-Id: Iad373fd41d34395e817e6de50dd9e5842b2ef623
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204200
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@bcmills
Copy link
Contributor Author

bcmills commented Nov 19, 2019 •
edited
Loading

Still timing out frequently:
https://build.golang.org/log/92ec5a2b1e867f332c17d8040dc14dfeedb4a719

@bcmills
Copy link
Contributor Author

bcmills commented Nov 19, 2019

This also masked an apparent regression in x/tools/gopls/test (#35691).

@matloob
Copy link
Contributor

matloob commented Dec 4, 2019

I'm not sure what we should do. Should we decrease the timeout?

@bcmills
Copy link
Contributor Author

bcmills commented Dec 5, 2019

Hmm, that last log only showed a signal: killed for golang.org/x/tools/go/packages, and the actual timeout was for golang.org/x/tools/internal/lsp/cmd.

So probably golang.org/x/tools/internal/lsp/cmd needs some timeouts. The failure for golang.org/x/tools/go/packages may have been an OOM condition instead — the co-occurrence with a failure fo x/tools/internal/lsp/cmd suggests a possible connection to #33951.

This was referenced Jan 7, 2020
Closed
Closed
@dmitshur
Copy link
Contributor

dmitshur commented Jan 7, 2020 •
edited
Loading

The failure for golang.org/x/tools/go/packages may have been an OOM condition instead — the co-occurrence with a failure fo x/tools/internal/lsp/cmd suggests a possible connection to #33951.

I've just filed #36444 for bringing the RAM of freebsd race builder closer to that of Linux and Windows. It's possible making that change will help with this issue.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/214433 mentions this issue: dashboard: upsize freebsd-amd64-race builder to 7.2 GB RAM

gopherbot pushed a commit to golang/build that referenced this issue Jan 14, 2020
@dmitshur
dashboard: upsize freebsd-amd64-race builder to 7.2 GB RAM
52f8f67 
Start using n1-highcpu-8 machine type instead of n1-highcpu-4
for the freebsd-amd64-race builder.

The freebsd-amd64-race builder has produced good test results
for the x/tools repo for a long time, but by now it has started
to consistently fail for reasons that seem connected to it having
only 3.6 GB memory. The Windows race builders needed to be bumped
from 7.2 GB to 14.4 GB to run successfully, so this change makes
a small incremental step to bring freebsd-amd64-race closer in
line with other builders. If memory-related problems continue to
occur with 7.2 GB, the next step will be to go up to 14.4 GB.

The freebsd-amd64-race builder is using an older version of FreeBSD.
We may want to start using a newer one for running tests with -race,
but that should be a separate change so we can see the results of
this change without another confounding variable.

Also update all FreeBSD builders to use https in buildletURLTmpl,
because it's expected to work fine and will be more consistent.

Updates golang/go#36444
Updates golang/go#34621
Updates golang/go#29252
Updates golang/go#33986

Change-Id: Idfcefd1c91bddc9f70ab23e02fcdca54fda9d1ac
Reviewed-on: https://go-review.googlesource.com/c/build/+/214433
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
@gopherbot

This comment has been minimized.

Sign in to view
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/227859 mentions this issue: dashboard: upsize freebsd-amd64-race builder to 16 vCPUs, 14.4 GB RAM

gopherbot pushed a commit to golang/build that referenced this issue Apr 10, 2020
@dmitshur
dashboard: upsize freebsd-amd64-race builder to 16 vCPUs, 14.4 GB RAM
baa2dcb 
This is a followup to CL 214433. Start using n1-highcpu-16 machine type
instead of n1-highcpu-8 for the freebsd-amd64-race builder.

Increasing the RAM from 3.6 GB to 7.2 GB has helped golang/go#36444
significantly: the builder stopped failing consistently on x/tools
and resulted in many data races being uncovered in golang/go#36605.

However, by now, it has started to fail consistently again. This
time it seems to be due to low performance, causing the tests in
golang.org/x/tools/internal/lsp/regtest package to fail due with
"context deadline exceeded" errors.

FreeBSD is one of the ports that stays visible when "show only first-
class ports" is checked on build.golang.org. The other -race builders
have all been upgraded to the n1-highcpu-16 machine type by now out
of necessity.

It seems fair to provide the FreeBSD port with an equal amount of
resources, even if the increased memory isn't strictly required yet.
Once this change is applied, if the failures persist, we can be more
confident that the problem is due to the code or the port, rather
than due to this -race builder having 2𝗑 less CPU and RAM resources
compared to other -race builders.

An alternative is to increase timeout for this builder type, but I'm
opting to defer exploring that after equalizing the machine type.

For golang/go#36444.
For golang/go#34621.
For golang/go#29252.
For golang/go#33986.

Change-Id: I41f149365128c7bc6f576c778ac07618acc04612
Reviewed-on: https://go-review.googlesource.com/c/build/+/227859
Reviewed-by: Alexander Rakoczy <alex@golang.org>
@dmitshur
Copy link
Contributor

dmitshur commented Jul 7, 2020 •
edited
Loading

I believe this has been fixed by the builder change in CL 227859. There have not been timeout errors in the x/tools/go/packages package tests lately. Closing.

This is a good data point for #33986.

@dmitshur dmitshur closed this as completed Jul 7, 2020
@golang golang locked and limited conversation to collaborators Jul 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) 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. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@dmitshur @bcmills @gopherbot @matloob