Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ikhoon committed Jun 10, 2024
1 parent 422f30e commit 8e34185
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ final boolean tryInitialize() {
"Can't send requests. ID: " + id + ", session active: " +
session.isAcquirable(responseDecoder.keepAliveHandler()));
}
session.deactivate();
session.markUnacquirable();
// No need to send RST because we didn't send any packet and this will be disconnected anyway.
fail(UnprocessedRequestException.of(exception));
return false;
Expand Down Expand Up @@ -223,7 +223,7 @@ final void writeHeaders(RequestHeaders headers) {
// connection by sending a GOAWAY frame that will be sent after receiving the corresponding
// response from the remote peer. The "Connection: close" header is stripped when it is converted to
// a Netty HTTP/2 header.
session.deactivate();
session.markUnacquirable();
}

final ChannelPromise promise = ch.newPromise();
Expand Down Expand Up @@ -329,9 +329,11 @@ private void fail(Throwable cause) {
}

final void failAndReset(Throwable cause) {
// Mark the session as unhealthy so that subsequent requests do not use it.
final HttpSession session = HttpSession.get(ch);
session.deactivate();
if (cause instanceof WriteTimeoutException) {
final HttpSession session = HttpSession.get(ch);
// Mark the session as unhealthy so that subsequent requests do not use it.
session.markUnacquirable();
}

if (cause instanceof ProxyConnectException || cause instanceof ResponseCompleteException) {
// - ProxyConnectException is handled by HttpSessionHandler.exceptionCaught().
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
}

if (!HttpUtil.isKeepAlive(nettyRes)) {
session().deactivate();
session().markUnacquirable();
}

final HttpResponseWrapper res = getResponse(resId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,14 @@ public void onStreamRemoved(Http2Stream stream) {}

@Override
public void onGoAwaySent(int lastStreamId, long errorCode, ByteBuf debugData) {
session().deactivate();
session().markUnacquirable();
goAwayHandler.onGoAwaySent(channel(), lastStreamId, errorCode, debugData);
}

@Override
public void onGoAwayReceived(int lastStreamId, long errorCode, ByteBuf debugData) {
// Should not reuse a connection that received a GOAWAY frame.
session().deactivate();
session().markUnacquirable();
goAwayHandler.onGoAwayReceived(channel(), lastStreamId, errorCode, debugData);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ private static final class ReadSuppressingAndChannelDeactivatingHandler extends

@Override
public void close(ChannelHandlerContext ctx, ChannelPromise promise) throws Exception {
HttpSession.get(ctx.channel()).deactivate();
HttpSession.get(ctx.channel()).markUnacquirable();
super.close(ctx, promise);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ public boolean isAcquirable(KeepAliveHandler keepAliveHandler) {
}

@Override
public void deactivate() {
public void markUnacquirable() {
isAcquirable = false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
}

if (!HttpUtil.isKeepAlive(nettyRes)) {
session().deactivate();
session().markUnacquirable();
}

if (res == null && ArmeriaHttpUtil.isRequestTimeoutResponse(nettyRes)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@ public CompletableFuture<Void> initiateConnectionShutdown() {
});
// To deactivate the channel when initiateShutdown is called after the RequestHeaders is sent.
// The next request will trigger shutdown.
HttpSession.get(ch).deactivate();
HttpSession.get(ch).markUnacquirable();
}
});
return completableFuture;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.linecorp.armeria.internal.client;

import com.linecorp.armeria.client.ClientRequestContext;
import com.linecorp.armeria.client.WriteTimeoutException;
import com.linecorp.armeria.common.ClosedSessionException;
import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.SerializationFormat;
Expand Down Expand Up @@ -90,7 +91,7 @@ public boolean isAcquirable(KeepAliveHandler keepAliveHandler) {
}

@Override
public void deactivate() {}
public void markUnacquirable() {}

@Override
public int incrementAndGetNumRequestsSent() {
Expand Down Expand Up @@ -137,9 +138,10 @@ static HttpSession get(Channel ch) {
* <li>A connection is closed.</li>
* <li>"Connection: close" header is sent or received.</li>
* <li>A GOAWAY frame is sent or received.</li>
* <li>A {@link WriteTimeoutException} is raised</li>
* </ul>
*/
void deactivate();
void markUnacquirable();

/**
* Returns {@code true} if a new request can be sent with this {@link HttpSession}.
Expand Down

0 comments on commit 8e34185

Please sign in to comment.