Skip to content

Commit 86342af

Browse files
committed
Addressed next round of review feedback:
- added test to check for close logging, with and without exception - fixed up close logger formatting - javadoc fixup for channel exception field - further refactored remaining code with throwable -> exception in Netty4Transport
1 parent 3ffdba7 commit 86342af

File tree

4 files changed

+56
-9
lines changed

4 files changed

+56
-9
lines changed

modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/transport/netty4/ESLoggingHandlerIT.java

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,21 @@
1111

1212
import org.apache.logging.log4j.Level;
1313
import org.elasticsearch.ESNetty4IntegTestCase;
14+
import org.elasticsearch.action.ActionListener;
1415
import org.elasticsearch.core.TimeValue;
1516
import org.elasticsearch.test.ESIntegTestCase;
1617
import org.elasticsearch.test.MockLog;
1718
import org.elasticsearch.test.junit.annotations.TestLogging;
19+
import org.elasticsearch.transport.ConnectionManager;
1820
import org.elasticsearch.transport.TcpTransport;
21+
import org.elasticsearch.transport.Transport;
22+
import org.elasticsearch.transport.TransportConnectionListener;
1923
import org.elasticsearch.transport.TransportLogger;
24+
import org.elasticsearch.transport.TransportService;
2025

2126
import java.io.IOException;
27+
import java.util.concurrent.CountDownLatch;
28+
import java.util.concurrent.TimeUnit;
2229

2330
@ESIntegTestCase.ClusterScope(numDataNodes = 2, scope = ESIntegTestCase.Scope.TEST)
2431
public class ESLoggingHandlerIT extends ESNetty4IntegTestCase {
@@ -96,7 +103,7 @@ public void testConnectionLogging() throws IOException {
96103
"close connection log",
97104
TcpTransport.class.getCanonicalName(),
98105
Level.DEBUG,
99-
".*closed transport connection \\[[1-9][0-9]*\\] to .* with age \\[[0-9]+ms\\].*"
106+
".*closed transport connection \\[[1-9][0-9]*\\] to .* with age \\[[0-9]+ms\\]$"
100107
)
101108
);
102109

@@ -105,4 +112,46 @@ public void testConnectionLogging() throws IOException {
105112

106113
mockLog.assertAllExpectationsMatched();
107114
}
115+
116+
@TestLogging(
117+
value = "org.elasticsearch.transport.TcpTransport:DEBUG",
118+
reason = "to ensure we log exception disconnect events on DEBUG level"
119+
)
120+
public void testExceptionalDisconnectLogging() throws Exception {
121+
mockLog.addExpectation(
122+
new MockLog.PatternSeenEventExpectation(
123+
"exceptional close connection log",
124+
TcpTransport.class.getCanonicalName(),
125+
Level.DEBUG,
126+
".*closed transport connection \\[[1-9][0-9]*\\] to .* with age \\[[0-9]+ms\\], exception:.*"
127+
)
128+
);
129+
130+
final String nodeName = internalCluster().startNode();
131+
132+
final CountDownLatch latch = new CountDownLatch(1);
133+
String masterNode = internalCluster().getMasterName();
134+
ConnectionManager connManager = internalCluster().getInstance(TransportService.class, masterNode).getConnectionManager();
135+
connManager.addListener(new TransportConnectionListener() {
136+
@Override
137+
public void onConnectionClosed(Transport.Connection conn) {
138+
conn.addCloseListener(new ActionListener<>() {
139+
@Override
140+
public void onResponse(Void ignored) {}
141+
142+
@Override
143+
public void onFailure(Exception e) {
144+
latch.countDown();
145+
}
146+
});
147+
}
148+
});
149+
150+
int failAttempts = 0;
151+
do {
152+
internalCluster().restartNode(nodeName);
153+
} while (latch.await(500, TimeUnit.MILLISECONDS) == false && failAttempts++ < 10);
154+
155+
mockLog.assertAllExpectationsMatched();
156+
}
108157
}

modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4TcpChannel.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ public class Netty4TcpChannel implements TcpChannel {
3434
private final ListenableFuture<Void> closeContext = new ListenableFuture<>();
3535
private final ChannelStats stats = new ChannelStats();
3636
private final boolean rstOnClose;
37-
// Exception causing a close, reported to the closeContext listener
37+
/**
38+
* Exception causing a close, reported to the {@link #closeContext} listener
39+
*/
3840
private volatile Exception closeException = null;
3941

4042
Netty4TcpChannel(Channel channel, boolean isServer, String profile, boolean rstOnClose, ChannelFuture connectFuture) {

modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ protected void stopInternal() {
307307
}, serverBootstraps::clear, () -> clientBootstrap = null);
308308
}
309309

310-
private Exception exceptionFromThrowable(Throwable cause) {
310+
static Exception exceptionFromThrowable(Throwable cause) {
311311
if (cause instanceof Error) {
312312
return new Exception(cause);
313313
} else {
@@ -399,11 +399,7 @@ private static class ServerChannelExceptionHandler extends ChannelInboundHandler
399399
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
400400
ExceptionsHelper.maybeDieOnAnotherThread(cause);
401401
Netty4TcpServerChannel serverChannel = ctx.channel().attr(SERVER_CHANNEL_KEY).get();
402-
if (cause instanceof Error) {
403-
onServerException(serverChannel, new Exception(cause));
404-
} else {
405-
onServerException(serverChannel, (Exception) cause);
406-
}
402+
onServerException(serverChannel, exceptionFromThrowable(cause));
407403
}
408404
}
409405
}

server/src/main/java/org/elasticsearch/transport/TcpTransport.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1208,7 +1208,7 @@ public void onFailure(Exception e) {
12081208
long closeTimeMillis = threadPool.relativeTimeInMillis();
12091209
logger.debug(
12101210
() -> format(
1211-
"closed transport connection [{}] to [{}] with age [{}ms], exception:",
1211+
"closed transport connection [%d] to [%s] with age [%dms], exception:",
12121212
connectionId,
12131213
node,
12141214
closeTimeMillis - openTimeMillis

0 commit comments

Comments
 (0)