This issue is for tracking down a better root cause for two flaky goroutine leak profiler tests, and then either updating the tests or the goroutine leak profiler as needed. (Perhaps it is easier for humans to collaborate here than in the very large watchflake issues.)
Background
Two subtests of TestGoroutineLeakProfileConcurrency seem to be common flakes in LUCI:
- overlapping_profile_requests
- overlapping_goroutine_and_goroutine_leak_profile_requests
Watchflake issues #76540 and #76749 currently have ~300 comments between them. A prior round of deflaking I think reduced the frequency, but watchflakes is reporting maybe a dozen or so a week recently between them (though I did not analyze prior or current rate too closely). The failure was also encountered by @FiloSottile in umbrella issue #79409.
I sent CL 778620 as a stop gap to possibly help in the near term, but this issue here is for a more complete solution.
Some initial theories
In #76540 (comment), Keith wrote:
This appears to be a goroutine coming back to life. On some profile requests, we get 6 leaked goroutines. Occasionally, we get only 5. (It's not actually combing back to life. It's just that the runtime occasionally thinks it might.)
I think what is going on here is that the underlying channel is occasionally kept alive by conservative scanning. I don't have strong evidence for this, but here's some weak evidence:
- It's nondeterministic. Calling goroutineLeakProf.WriteTo(&w, 1) twice in a row, sometimes the first result has 5 leaked goroutines and the second has 6. Sometimes vice versa.
- If I pair up the leaking goroutines (2 goroutines each reading from a dead channel), then when it fails there are 4 leaked goroutines instead of 6.
- If I remove this loop (so that a goroutine getting marked as leaked is sticky), it fixes the problem.
[...]
Maybe making leaked a sticky state would be a reasonable fix. I think it needs more than what I just did in number 3, but maybe it wouldn't be too bad.
In the discussion on the CL 778620 that I sent, Vlad wrote:
This is definitely something to dig into. I suspect a small race condition around goroutine statuses at the beginning/end of a leak profile cycle when there are too many concurrent requests (though the off-by-1 seems too suspiciously consistent for that to be the case).
CC @VladSaiocUber @randall77 , @prattmic, @dmitshur
This issue is for tracking down a better root cause for two flaky goroutine leak profiler tests, and then either updating the tests or the goroutine leak profiler as needed. (Perhaps it is easier for humans to collaborate here than in the very large watchflake issues.)
Background
Two subtests of TestGoroutineLeakProfileConcurrency seem to be common flakes in LUCI:
Watchflake issues #76540 and #76749 currently have ~300 comments between them. A prior round of deflaking I think reduced the frequency, but watchflakes is reporting maybe a dozen or so a week recently between them (though I did not analyze prior or current rate too closely). The failure was also encountered by @FiloSottile in umbrella issue #79409.
I sent CL 778620 as a stop gap to possibly help in the near term, but this issue here is for a more complete solution.
Some initial theories
In #76540 (comment), Keith wrote:
In the discussion on the CL 778620 that I sent, Vlad wrote:
CC @VladSaiocUber @randall77 , @prattmic, @dmitshur