Switch to NIO #195

acogoluegnes · 2016-09-07T12:50:09Z

Fixes #11

WIP Fixes #11

The current limit may be arbitrary right now (depends on the hardware and the scenario). Write as much as possible in the ByteBuffer before clearing it. Fixes #11

Fixes #11

Limit also written frames to the (snapshot) size when iteration starts (instead of unqueuing during writing). Fixes #11

Fixes #11

Use NIO by default (except in SSLTests). Fixes #11

Fixes #11

And set number of NIO threads a parameter. Fixes #11

Fixes #11

Allows to properly handle the first send header request. Fixes #11

Conflicts: src/main/java/com/rabbitmq/client/impl/ForgivingExceptionHandler.java src/main/java/com/rabbitmq/client/impl/SocketFrameHandler.java

First draft. Fixes #11

Conflicts: src/main/java/com/rabbitmq/client/impl/recovery/AutorecoveringConnection.java

Using ByteBuffer-based streams, to avoid duplicating the frame parsing/write logic. Fixes #11

Fixes #11

Clean some unused code, handle connection and TLS handshake failure, add configuration hook for SocketChannel, create 'use*' methods to activate NIO or blocking IO. Fixes #11

Fixes #11

michaelklishin · 2016-09-28T09:53:43Z

This fails CI and no longer merges cleanly.

Conflicts: src/test/java/com/rabbitmq/examples/PerfTest.java

Fixes #11

This test was initially created to reproduce a transient error during the TLS handshake. It looks like this issue was related to Erlang, introduced in 19.x and fixed in 19.1.1 (http://erlang.org/download/OTP-19.1.1.README). Fixes #11

michaelklishin

With this PR I have CPU load hovering around 600% when running mvn verify. At the end tests begin to fail with java.lang.OutOfMemoryError: unable to create new native thread. So my guess is that there is a thread instance leak.

The ExecutorService wasn't closed properly in the base test class, there were thus many threads in park state when running the test suite. This could lead to resource exhaustion, especially on MacOS. Fixes #11

On MacOS, SocketChannel.read() can return EOF when the SocketChannel is closed. On Linux, it would throw an exception. This could lead to spinning threads on MacOS, because the NIO loop was never closed. Fixes #11

The default is to use blocking IO. Fixes #11

michaelklishin · 2016-10-05T14:04:25Z

With the recent EOF handling fix, the CPU/Heap/Threads charts in Visual VM over the course of a test suite run now look nearly identical with the default profile vs. use-nio. This is looking good for inclusion as an opt-in feature for 4.0.0.

michaelklishin · 2016-10-05T14:05:35Z

src/main/java/com/rabbitmq/client/impl/nio/NioHelper.java

+    static int read(ReadableByteChannel channel, ByteBuffer buffer) throws IOException {
+        int read = channel.read(buffer);
+        if(read < 0) {
+            throw new IOException("Channel has reached EOF");


This is confusing. The users will think it has to do with RabbitMQ channels. Please use "Connection has reached EOF" or simply "I/O thread: reached EOF".

michaelklishin · 2016-10-05T14:07:25Z

src/main/java/com/rabbitmq/client/impl/nio/NioParams.java

+public class NioParams {
+
+    /** size of the byte buffer used for inbound data */
+    private int readByteBufferSize = 8192;


I'd bump this and the write buffer here to 32K, it sounds quite reasonable (default TCP buffer size on Linux and OS X is around 100K, for instance).

michaelklishin · 2016-10-05T14:32:44Z

I observe a couple of test failures in TLS with NIO enabled:

17:25:51.472 [main] INFO  c.r.client.test.BrokerTestCase - Test finished: NioTlsUnverifiedConnection.connectionGetConsume
Tests run: 8, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.392 sec <<< FAILURE! - in com.rabbitmq.client.test.ssl.SSLTests
sSL(com.rabbitmq.client.test.ssl.VerifiedConnection)  Time elapsed: 0.232 sec  <<< FAILURE!
java.lang.AssertionError: Couldn't open TLS connection after 3 attemps
    at com.rabbitmq.client.test.ssl.VerifiedConnection.openConnection(VerifiedConnection.java:99)

sSL(com.rabbitmq.client.test.ssl.VerifiedConnection)  Time elapsed: 0.232 sec  <<< FAILURE!
java.lang.AssertionError: Couldn't open TLS connection after 3 attemps
    at com.rabbitmq.client.test.ssl.VerifiedConnection.openConnection(VerifiedConnection.java:99)


Results :

Failed tests:
com.rabbitmq.client.test.ssl.VerifiedConnection.sSL(com.rabbitmq.client.test.ssl.VerifiedConnection)
  Run 1: VerifiedConnection>BrokerTestCase.setUp:71->openConnection:99 Couldn't open TLS connection after 3 attemps
  Run 2: VerifiedConnection>BrokerTestCase.tearDown:82->openConnection:99 Couldn't open TLS connection after 3 attemps

(on OS X)

And change EOF message, to make it clearer. Fixes #11

acogoluegnes · 2016-10-05T14:53:43Z

The TLS handshake failures are likely to happen with Erlang 19.x. It's been fixed in 19.1.1. These failures are transient, with blocking IO and NIO.

michaelklishin · 2016-10-05T16:41:20Z

Yes, that was it. The suite passes with flying colors on 19.1.1.

acogoluegnes added 10 commits September 7, 2016 14:46

Implement basic NIO support

7af1936

Fixes #11

Fix loops and IO frame reading

ceb9caf

Fixes #11

Fix connection sequence

ea1d67a

Fixes #11

Use only NIO threads

482449a

WIP Fixes #11

Set a limit in write queue

f695768

The current limit may be arbitrary right now (depends on the hardware and the scenario). Write as much as possible in the ByteBuffer before clearing it. Fixes #11

Improve frame handling

a682f45

Fixes #11

Fix connection

fe2ecff

Fixes #11

Handle heartbeat and shutdown

5c080c0

Limit also written frames to the (snapshot) size when iteration starts (instead of unqueuing during writing). Fixes #11

Handle connection shutdown in IO loops

ef7eeb5

Fixes #11

Create FrameHandlerFactory interface

6335aa4

Fixes #11

michaelklishin self-assigned this Sep 12, 2016

acogoluegnes added 19 commits September 13, 2016 13:05

Clean NIO state automatically

ed3deab

Use NIO by default (except in SSLTests). Fixes #11

Polish

2a638c8

Fixes #11

Add NIO params

554dc6a

And set number of NIO threads a parameter. Fixes #11

Merge branch 'master' into rabbitmq-java-client-11

85f6737

Re-organize NIO classes

0386b4e

Fixes #11

Add WriteRequest abstraction

2d337d9

Allows to properly handle the first send header request. Fixes #11

Merge branch 'master' into rabbitmq-java-client-11

3f87858

Conflicts: src/main/java/com/rabbitmq/client/impl/ForgivingExceptionHandler.java src/main/java/com/rabbitmq/client/impl/SocketFrameHandler.java

Handle TLS in NIO

65cfd4d

First draft. Fixes #11

Merge branch 'master' into rabbitmq-java-client-11

4bec5d1

Conflicts: src/main/java/com/rabbitmq/client/impl/recovery/AutorecoveringConnection.java

Use streams in plain NIO loops

d6a7478

Using ByteBuffer-based streams, to avoid duplicating the frame parsing/write logic. Fixes #11

Use same logic for plain/TLS in IO loops

44684c9

Fixes #11

Merge branch 'master' into rabbitmq-java-client-11

c554cf9

Close SSL engine on channel closing

e533072

Fixes #11

Merge branch 'master' into rabbitmq-java-client-11

22df994

Check SocketChannel is open before reading/writing

4083b65

Fixes #11

Use one thread for NIO

2826752

Fixes #11

Add tests for TLS on NIO

0b8c4dd

Fixes #11

Document NIO API

5dba9cc

Clean some unused code, handle connection and TLS handshake failure, add configuration hook for SocketChannel, create 'use*' methods to activate NIO or blocking IO. Fixes #11

Fix TLS handshake

5717a22

Fixes #11

Improve error messages

0532439

Fixes #11

acogoluegnes changed the title ~~DO NOT MERGE Switch to NIO~~ Switch to NIO Sep 26, 2016

acogoluegnes added 10 commits September 28, 2016 12:01

Merge branch 'master' into rabbitmq-java-client-11

3a1320e

Conflicts: src/test/java/com/rabbitmq/examples/PerfTest.java

Add TLS handshake test

3a3eafb

Add TLS handshake test to test suite

ee94a9e

Fixes #11

Shutdown NIO loop ASAP

6168e97

Fixes #11

Add connection re-attemps to stabilize SSL tests

9f3fe90

Fixes #11

Repeat reading from socket when reading frame

33cd661

Fixes #11

Merge branch 'master' into rabbitmq-java-client-11

e680acd

Check Connection availability in NIO loop

278c835

Fixes #11

Merge branch 'master' into rabbitmq-java-client-11

2f19ad0

Remove TLS handshake looping test

22bf675

This test was initially created to reproduce a transient error during the TLS handshake. It looks like this issue was related to Erlang, introduced in 19.x and fixed in 19.1.1 (http://erlang.org/download/OTP-19.1.1.README). Fixes #11

michaelklishin requested changes Oct 4, 2016

View reviewed changes

acogoluegnes added 3 commits October 4, 2016 14:02

Use default ThreadFactory for NIO in test suite

d74b125

The ExecutorService wasn't closed properly in the base test class, there were thus many threads in park state when running the test suite. This could lead to resource exhaustion, especially on MacOS. Fixes #11

Handle EOF properly in NIO

6d912dd

On MacOS, SocketChannel.read() can return EOF when the SocketChannel is closed. On Linux, it would throw an exception. This could lead to spinning threads on MacOS, because the NIO loop was never closed. Fixes #11

Add use-nio profile for Maven test suite

b652b00

The default is to use blocking IO. Fixes #11

michaelklishin reviewed Oct 5, 2016

View reviewed changes

michaelklishin approved these changes Oct 5, 2016

View reviewed changes

Bump NIO buffers to 32 K

9a8db70

And change EOF message, to make it clearer. Fixes #11

michaelklishin merged commit 65367b5 into master Oct 5, 2016

acogoluegnes deleted the rabbitmq-java-client-11 branch March 21, 2019 08:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Switch to NIO #195

Switch to NIO #195

Uh oh!

acogoluegnes commented Sep 7, 2016

Uh oh!

michaelklishin commented Sep 28, 2016

Uh oh!

michaelklishin left a comment

Uh oh!

michaelklishin commented Oct 5, 2016

Uh oh!

michaelklishin Oct 5, 2016

Uh oh!

michaelklishin Oct 5, 2016

Uh oh!

michaelklishin commented Oct 5, 2016

Uh oh!

acogoluegnes commented Oct 5, 2016

Uh oh!

michaelklishin commented Oct 5, 2016

Uh oh!

Uh oh!

Switch to NIO #195

Switch to NIO #195

Uh oh!

Conversation

acogoluegnes commented Sep 7, 2016

Uh oh!

michaelklishin commented Sep 28, 2016

Uh oh!

michaelklishin left a comment

Choose a reason for hiding this comment

Uh oh!

michaelklishin commented Oct 5, 2016

Uh oh!

michaelklishin Oct 5, 2016

Choose a reason for hiding this comment

Uh oh!

michaelklishin Oct 5, 2016

Choose a reason for hiding this comment

Uh oh!

michaelklishin commented Oct 5, 2016

Uh oh!

acogoluegnes commented Oct 5, 2016

Uh oh!

michaelklishin commented Oct 5, 2016

Uh oh!

Uh oh!