Thanks for the review. I merged the PR and triggered a manual build https://github.com/apache/pulsar/actions/runs/6690374946 to get the latest report of leaked threads.
-Lari On Mon, 30 Oct 2023, 9.15 Enrico Olivelli, <eolive...@gmail.com> wrote: > Il Lun 30 Ott 2023, 06:39 Lari Hotari <lhot...@apache.org> ha scritto: > > > Hi Asaf, > > > > Yes, the visibility aspect is already solved by using warnings in the > > summary view. Please check the example > > https://github.com/apache/pulsar/actions/runs/6680066364?pr=21450 . > > > > Job summaries could also be used, but they have less visibility in the > > summary view, as you can see from the example. Job summaries are on > placed > > on the summary page after errors/warnings and build artifacts and when > > there are more than a few summaries, each job summary will need to be > > explicitly expanded by clicking "Load Summary" to view the content. That > > makes their visibility lower than warnings. > > > > Since this is a change in the build and isn't really intrusive, I think > we > > could get it merged and revisit it based on the experiences we get from > the > > use of it. I have been iterating on the solution while fixing a lot of > the > > test resource leaks in the last few weeks. Without support for detecting > > the resource leaks, it's really hard to keep the test suite clean. > > > > > > > Looking forward to more reviews on > > https://github.com/apache/pulsar/pull/21450 . :) > > > > > Looks great > > Thanks > Enrico > > > > > > > -Lari > > > > On 2023/10/29 18:34:28 Asaf Mesika wrote: > > > Larry, I know there is a way to add like a Job summary, so we can write > > it > > > there - do you think this can increase visibility? > > > > > > On Sun, Oct 29, 2023 at 4:53 AM Lari Hotari <lhot...@apache.org> > wrote: > > > > > > > Hi all, > > > > > > > > I have submitted a PR (https://github.com/apache/pulsar/pull/21450) > > which > > > > includes changes to add reporting and tooling to detect thread leaks > in > > > > Pulsar tests. > > > > > > > > It should be ensured in each test that resources created by the test > > are > > > > properly cleaned up. Failing to do so can lead to memory leaks and, > in > > some > > > > instances, unnecessary CPU consumption. These issues can, in turn, > slow > > > > down test execution, increase Pulsar CI build durations, and cause > > > > flakiness. A significant source of memory leaks in Pulsar tests > stems > > from > > > > thread leaks. > > > > > > > > After the PR is merged, it will be easy to detect thread leaks since > > the > > > > build will add warnings to the summary view for the GitHub Actions > > build > > > > run. An example can be seen in the PR build run: > > > > https://github.com/apache/pulsar/actions/runs/6680066364?pr=21450 . > > > > There will be more detailed information in the "Report detected > thread > > > > leaks" build step, for example > > > > > > > https://github.com/apache/pulsar/actions/runs/6680066364/job/18153890519?pr=21450#step:16:23 > > > > . > > > > > > > > Please review the PR https://github.com/apache/pulsar/pull/21450 so > > that > > > > we can continue to get rid of the remaining thread leaks in the > future > > and > > > > keep the tests cleaner and less flaky. > > > > > > > > -Lari > > > > > > > > > >
