Skip to content

Conversation

@mhl-b
Copy link
Contributor

@mhl-b mhl-b commented Apr 12, 2025

Release leaking buffers in Netty4HttpHeaderValidatorTests.

Closes #126743
Closes #126745
Closes #126749

@mhl-b mhl-b added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/Network Http and internode communication implementations Team:Distributed Coordination Meta label for Distributed Coordination team v9.1.0 labels Apr 12, 2025
@mhl-b mhl-b requested a review from DaveCTurner April 12, 2025 07:02
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM with optional suggestions

Comment on lines 143 to 146
var lastContent = channel.readInbound();
assertTrue(lastContent instanceof LastHttpContent);
((LastHttpContent) lastContent).release();

Copy link
Contributor

Choose a reason for hiding this comment

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

Or, in one line (with slightly nicer failure message too):

Suggested change
var lastContent = channel.readInbound();
assertTrue(lastContent instanceof LastHttpContent);
((LastHttpContent) lastContent).release();
asInstanceOf(LastHttpContent.class, channel.readInbound()).release();

Comment on lines 165 to 167
var fullReq = channel.readInbound();
assertTrue(fullReq instanceof FullHttpRequest);
((FullHttpRequest) fullReq).release();
Copy link
Contributor

Choose a reason for hiding this comment

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

Likewise

Suggested change
var fullReq = channel.readInbound();
assertTrue(fullReq instanceof FullHttpRequest);
((FullHttpRequest) fullReq).release();
asInstanceOf(FullHttpRequest.class, channel.readInbound()).release();

@mhl-b mhl-b merged commit f9f3def into elastic:main Apr 14, 2025
17 checks passed
brianseeders added a commit to brianseeders/elasticsearch that referenced this pull request Apr 17, 2025
bcully added a commit to bcully/elasticsearch that referenced this pull request Apr 17, 2025
bcully pushed a commit that referenced this pull request Apr 17, 2025
…ipeline (#127030)

* Revert "Release buffers in netty test (#126744)"

This reverts commit f9f3def.

* Revert "Add flow-control and remove auto-read in netty4 HTTP pipeline (#126441)"

This reverts commit c8805b8.
ankikuma added a commit to ankikuma/elasticsearch that referenced this pull request Apr 17, 2025
This reverts commit f9f3def.

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

Labels

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

Projects

None yet

3 participants