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

runtime/pprof: TestLabelSystemstack due to sample with no location [1.19 backport] #58939

Closed
gopherbot opened this issue Mar 8, 2023 · 2 comments
Closed

runtime/pprof: TestLabelSystemstack due to sample with no location [1.19 backport] #58939

gopherbot opened this issue Mar 8, 2023 · 2 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.19.8

Comments

@gopherbot
Copy link
Contributor

@prattmic requested issue #51550 to be considered for backport to the next 1.19 minor release.

@gopherbot Please backport to 1.19. This is a fix for a flaky test, which is also flaky on 1.19.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Mar 8, 2023
@gopherbot gopherbot mentioned this issue Mar 8, 2023
Open
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Mar 8, 2023
@gopherbot gopherbot added this to the Go1.19.8 milestone Mar 8, 2023
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/474618 mentions this issue: [release-branch.go1.19] runtime/pprof: improve output of TestLabelSystemstack
@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Mar 8, 2023
@cherrymui cherrymui added the CherryPickApproved Used during the release process for point releases label Mar 15, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Mar 15, 2023
gopherbot pushed a commit that referenced this issue Mar 15, 2023
@aclements @cherrymui
[release-branch.go1.19] runtime/pprof: improve output of TestLabelSys…
3a50af1 
…temstack

The current output of TestLabelSystemstack is a bit cryptic. This CL
improves various messages and hopefully simplifies the logic in the
test.

Simplifying the logic leads to three changes in possible outcomes,
which I verified by running the logic before and after this change
through all 2^4 possibilities (https://go.dev/play/p/bnfb-OQCT4j):

1. If a sample both must be labeled and must not be labeled, the test
now reports that explicitly rather than giving other confusing output.

2. If a sample must not be labeled but is, the current logic will
print two identical error messages. The new logic prints only one.

3. If the test finds no frames at all that it recognizes, but the
sample is labeled, it will currently print a confusing "Sample labeled
got true want false" message. The new logic prints nothing. We've seen
this triggered by empty stacks in profiles.

Fixes #51550. This bug was caused by case 3 above, where it was
triggered by a profile label on an empty stack. It's valid for empty
stacks to appear in a profile if we sample a goroutine just as it's
exiting (and that goroutine may have a profile label), so the test
shouldn't fail in this case.

For #58939.

Change-Id: I1593ec4ac33eced5bb89572a3ba7623e56f2fb3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/460516
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit d9f23cf)
Reviewed-on: https://go-review.googlesource.com/c/go/+/474618
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
@gopherbot
Copy link
Contributor Author

Closed by merging 3a50af1 to release-branch.go1.19.
@golang golang locked and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge Testing An issue that has been verified to require only test changes, not just a test failure.
3 participants
@bcmills @gopherbot @cherrymui