Skip to content

Add leak tracking and unmute Netty4ChunkedContinuationsIT#110175

Merged
nicktindall merged 3 commits intoelastic:mainfrom
nicktindall:fix/109866_add_logging_and_unmute_chunkedcontinuationstest
Jun 26, 2024
Merged

Add leak tracking and unmute Netty4ChunkedContinuationsIT#110175
nicktindall merged 3 commits intoelastic:mainfrom
nicktindall:fix/109866_add_logging_and_unmute_chunkedcontinuationstest

Conversation

@nicktindall
Copy link
Copy Markdown
Contributor

Related to: #109866

I've had great difficulty in reproducing this failure, this change should give us more information about where the issue is

@nicktindall nicktindall marked this pull request as ready for review June 26, 2024 03:17
@nicktindall nicktindall added >test Issues or PRs that are addressing/adding tests :Distributed/Network Http and internode communication implementations labels Jun 26, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Meta label for distributed team. label Jun 26, 2024
} finally {
refs = null;
reachabilityChecker.ensureUnreachable();
}
Copy link
Copy Markdown
Contributor Author

@nicktindall nicktindall Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LeakTracker will log the sequence of incRef and decRef that resulted in the failure, the ReachabilityChecker will ensure we wait for the LeakTracker to kick in during the test.

Copy link
Copy Markdown
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

…ticsearch/http/netty4/Netty4ChunkedContinuationsIT.java

Co-authored-by: Yang Wang <ywangd@gmail.com>
@nicktindall nicktindall merged commit caf4bc1 into elastic:main Jun 26, 2024
@nicktindall nicktindall deleted the fix/109866_add_logging_and_unmute_chunkedcontinuationstest branch June 26, 2024 05:17
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Jun 30, 2024
With the changes in elastic#109519 we now do one more async step while serving
the response, so we need to acquire another ref to track the new step.

Relates elastic#109866
Relates elastic#110118
Relates elastic#110175
Relates elastic#110249
DaveCTurner added a commit that referenced this pull request Jul 1, 2024
With the changes in #109519 we now do one more async step while serving
the response, so we need to acquire another ref to track the new step.

Relates #109866
Relates #110118
Relates #110175
Relates #110249
@DaveCTurner
Copy link
Copy Markdown
Member

FWIW this failure did reproduce for me locally, although it's not unusual for things like this to take a few hours of iterations to catch the culprit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Network Http and internode communication implementations Team:Distributed Meta label for distributed team. >test Issues or PRs that are addressing/adding tests v8.15.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants