Skip to content

Guard debug logs in pollPooledChannel #2300

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

Open
pavel-ptashyts wants to merge 2 commits into
base: main
Choose a base branch
from
+12 −4
Open

Guard debug logs in pollPooledChannel #2300

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a28edd
Guard debug logs in pollPooledChannel
pavel-ptashyts Aug 3, 2026
7dfcf19
Move the varargs note to the top of the method
pavel-ptashyts Aug 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1373,19 +1373,25 @@ private Channel pollPooledChannel(NettyResponseFuture<?> future, Request request
Uri uri = request.getUri();
String virtualHost = request.getVirtualHost();

// SLF4J has no three-argument overload, so every debug statement below binds to
// debug(String, Object...) and allocates its varargs array at the call site whatever the level. This
// is the steady-state connection-reuse path, so they are all guarded explicitly.

// Round-robin mode: poll with the IP-aware key so reuse stays pinned to the chosen IP (both the
// HTTP/2 registry and the HTTP/1.1 pool).
Object override = future != null ? future.getPartitionKeyOverride() : null;
if (override != null) {
if (!uri.isWebSocket()) {
Channel h2Channel = channelManager.pollHttp2Connection(override);
if (h2Channel != null) {
LOGGER.debug("Using HTTP/2 multiplexed Channel '{}' for '{}' to '{}'", h2Channel, request.getMethod(), uri);
if (LOGGER.isDebugEnabled()) {

@hyperxpro hyperxpro Aug 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Small correction to the description: the guard in sendRequestWithOpenChannel also hoists getNettyRequest().getHttpRequest() inside the branch, so it is not purely about the varargs array. The precedent stands, the reasoning is just a bit wider there.

@pavel-ptashyts pavel-ptashyts Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Corrected in the description. It hoists getNettyRequest().getHttpRequest() into the branch as well, so two calls are skipped on top of the array. The precedent still holds, the reasoning there is just wider than here.

LOGGER.debug("Using HTTP/2 multiplexed Channel '{}' for '{}' to '{}'", h2Channel, request.getMethod(), uri);
}
return h2Channel;
}
}
Channel channel = channelManager.poll(override);
if (channel != null) {
if (channel != null && LOGGER.isDebugEnabled()) {
LOGGER.debug("Using pooled Channel '{}' for '{}' to '{}'", channel, request.getMethod(), uri);
}
return channel;
Expand All @@ -1406,14 +1412,16 @@ private Channel pollPooledChannel(NettyResponseFuture<?> future, Request request
if (!uri.isWebSocket()) {
Channel h2Channel = channelManager.pollHttp2Connection(partitionKey);
if (h2Channel != null) {
LOGGER.debug("Using HTTP/2 multiplexed Channel '{}' for '{}' to '{}'", h2Channel, request.getMethod(), uri);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Using HTTP/2 multiplexed Channel '{}' for '{}' to '{}'", h2Channel, request.getMethod(), uri);
}
return h2Channel;
}
}

final Channel channel = channelManager.poll(partitionKey);

if (channel != null) {
if (channel != null && LOGGER.isDebugEnabled()) {

@hyperxpro hyperxpro Aug 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the common path and there is nothing here saying why the guard exists. Moving the comment up would cover it.

@pavel-ptashyts pavel-ptashyts Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same move covers this one. The note is above the override branch now, so it reads before either poll.

LOGGER.debug("Using pooled Channel '{}' for '{}' to '{}'", channel, request.getMethod(), uri);
}
return channel;
Expand Down
Loading