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

x/tools/gopls: various regtest flakes due to timeout during shutdown. #53820

Closed
findleyr opened this issue Jul 12, 2022 · 3 comments
Closed

x/tools/gopls: various regtest flakes due to timeout during shutdown. #53820

findleyr opened this issue Jul 12, 2022 · 3 comments
Assignees
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. 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

Comments

@findleyr
Copy link
Contributor

Following the change to properly shut down gopls after regression tests, we have a new flake for TestFormattingOneLineImports36824.
https://build.golang.org/log/7ac675121accf7fbc43a15601ab898040c7d9643

Goroutine dump shows that gopls is still awaiting a Go command.

Test shutdown had an arbitrary timeout of 5s. Now that we're waiting for ongoing work, this may sometimes be insufficient due to resource contention. We should start by increasing (or removing) this timeout to see if the flakes go away.

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jul 12, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jul 12, 2022
@findleyr findleyr added the Testing An issue that has been verified to require only test changes, not just a test failure. label Jul 14, 2022
@findleyr findleyr changed the title x/tools/gopls: TestFormattingOneLineImports36824 flake due to timeout during shutdown. x/tools/gopls: various regtest flakes due to timeout during shutdown. Jul 15, 2022
@findleyr
Copy link
Contributor Author

Seeing many more similar flakes in arbitrary tests (e.g. https://build.golang.org/log/9b4cda8292abc11d4807641e8af88e1cf57b2fd3).

Sending a CL to increase the timeout for now. We can also consider terminating the go command more aggressively when context is cancelled.

@gopherbot
Copy link

Change https://go.dev/cl/417583 mentions this issue: internal/lsp/regtest: increase the time allowed for shutdown

gopherbot pushed a commit to golang/tools that referenced this issue Jul 15, 2022
Now that we await ongoing work during shutdown, we are seeing regtest
flakes simply due to outstanding go command invocations.

Allow more time for cleanup. If this is insufficient, we can be more
aggressive about terminating go command processes when context is
cancelled.

For golang/go#53820

Change-Id: I3df3c5510dae34cb14a6efeb02c2963a71e64f3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417583
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dylan Le <dungtuanle@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
@findleyr
Copy link
Contributor Author

This seems to have been resolved by the increased deadline.

@findleyr findleyr self-assigned this Aug 8, 2022
@golang golang locked and limited conversation to collaborators Aug 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. 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
Development

No branches or pull requests

2 participants