Skip to content

Commit 9f970f2

Browse files
committed
Merge pull request #1534 from shapeblue/niotest-fix
CLOUDSTACK-9348: Optimize NioTest and NioConnection main loop- Reduces SSL handshake timeout to 15s, previously this was only 10s in commit debfcde - Adds an aggresive explicit wakeup to save the Nio main IO loop/handler from getting blocked - Fix NioTest to fail/succeed in about 60s, previously this was 300s - Due to aggresive wakeup usage, NioTest should complete in less than 5s on most systems. On virtualized environment this may slightly increase due to thread, CPU burst/scheduling delays. /cc @swill please review and merge. Sorry about the previous values, they were not optimized for virtualized env. The aggressive selector.wakeup will ensure main IO loop does not get blocked even by malicious users, even for any timeout (ssl handshake etc). * pr/1534: CLOUDSTACK-9348: Optimize NioTest and NioConnection main loop Signed-off-by: Will Stevens <williamstevens@gmail.com>
2 parents 6d0c92b + ea22869 commit 9f970f2

3 files changed

Lines changed: 18 additions & 11 deletions

File tree

utils/src/main/java/com/cloud/utils/nio/Link.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -596,8 +596,8 @@ public static boolean doHandshake(final SocketChannel socketChannel, final SSLEn
596596
while (handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED
597597
&& handshakeStatus != SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING) {
598598
final long timeTaken = System.currentTimeMillis() - startTimeMills;
599-
if (timeTaken > 60000L) {
600-
s_logger.warn("SSL Handshake has taken more than 60s to connect to: " + socketChannel.getRemoteAddress() +
599+
if (timeTaken > 15000L) {
600+
s_logger.warn("SSL Handshake has taken more than 15s to connect to: " + socketChannel.getRemoteAddress() +
601601
". Please investigate this connection.");
602602
return false;
603603
}

utils/src/main/java/com/cloud/utils/nio/NioConnection.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,8 @@ public Boolean call() throws NioConnectionException {
171171
} catch (final IOException e) {
172172
s_logger.error("Agent will die due to this IOException!", e);
173173
throw new NioConnectionException(e.getMessage(), e);
174+
} finally {
175+
_selector.wakeup();
174176
}
175177
}
176178
_isStartup = false;

utils/src/test/java/com/cloud/utils/testcase/NioTest.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,17 +61,17 @@ public class NioTest {
6161
private static final Logger LOGGER = Logger.getLogger(NioTest.class);
6262

6363
// Test should fail in due time instead of looping forever
64-
private static final int TESTTIMEOUT = 300000;
64+
private static final int TESTTIMEOUT = 60000;
6565

66-
final private int totalTestCount = 5;
66+
final private int totalTestCount = 4;
6767
private int completedTestCount = 0;
6868

6969
private NioServer server;
7070
private List<NioClient> clients = new ArrayList<>();
7171
private List<NioClient> maliciousClients = new ArrayList<>();
7272

7373
private ExecutorService clientExecutor = Executors.newFixedThreadPool(totalTestCount, new NamedThreadFactory("NioClientHandler"));;
74-
private ExecutorService maliciousExecutor = Executors.newFixedThreadPool(5*totalTestCount, new NamedThreadFactory("MaliciousNioClientHandler"));;
74+
private ExecutorService maliciousExecutor = Executors.newFixedThreadPool(totalTestCount, new NamedThreadFactory("MaliciousNioClientHandler"));;
7575

7676
private Random randomGenerator = new Random();
7777
private byte[] testBytes;
@@ -105,12 +105,18 @@ public void setUp() {
105105
Assert.fail(e.getMessage());
106106
}
107107

108+
/**
109+
* The malicious client(s) tries to block NioServer's main IO loop
110+
* thread until SSL handshake timeout value (from Link class, 15s) after
111+
* which the valid NioClient(s) get the opportunity to make connection(s)
112+
*/
113+
for (int i = 0; i < totalTestCount; i++) {
114+
final NioClient maliciousClient = new NioMaliciousClient("NioMaliciousTestClient-" + i, "127.0.0.1", server.getPort(), 1, new NioMaliciousTestClient());
115+
maliciousClients.add(maliciousClient);
116+
maliciousExecutor.submit(new ThreadedNioClient(maliciousClient));
117+
}
118+
108119
for (int i = 0; i < totalTestCount; i++) {
109-
for (int j = 0; j < 4; j++) {
110-
final NioClient maliciousClient = new NioMaliciousClient("NioMaliciousTestClient-" + i, "127.0.0.1", server.getPort(), 1, new NioMaliciousTestClient());
111-
maliciousClients.add(maliciousClient);
112-
maliciousExecutor.submit(new ThreadedNioClient(maliciousClient));
113-
}
114120
final NioClient client = new NioClient("NioTestClient-" + i, "127.0.0.1", server.getPort(), 1, new NioTestClient());
115121
clients.add(client);
116122
clientExecutor.submit(new ThreadedNioClient(client));
@@ -286,7 +292,6 @@ public void doTask(final Task task) {
286292
LOGGER.info("Server: Received OTHER task");
287293
}
288294
}
289-
290295
}
291296
}
292297
}

0 commit comments

Comments
 (0)