nats-io
Initial AsynchronousSocketChannel implementation.
Based off of #356. Doesn't yet support SSL but seems to work for the most part otherwise.
Is this PR still alive? I ask primarily since I'm proposing a DataPort refactor so that we can layer protocols such as websockets "underneath" the NATS protocol. Here is my proposal: https://github.com/nats-io/nats.java/pull/494
Note that we could add asynchronous I/O in the above PR if that is desired, since the above PR is already having to refactor the DataPort (but in a way that is mostly backwards compatible).
@brimworks I was planning to re-base this at some point but if you think you can incorporate AsynchronousSocketChannel based sockets into #494 as a starting point that may be easier as this is a good bit out of sync at the moment.
@jameshilliard is the main motivation to eventually allow the Connection to be used as a reactive stream publisher and subscriber? ...or are there other motivations (such as improved performance)? If it is improved performance, do we have a performance test that we can run to ensure that using async I/O indeed improves the performance (both memory and CPU)?
is the main motivation to eventually allow the
Connectionto be used as a reactive stream publisher and subscriber?
Kinda, I don't think I was targeting reactive streams so much as improved interop with netty based async java client/server libraries.
If it is improved performance, do we have a performance test that we can run to ensure that using async I/O indeed improves the performance (both memory and CPU)?
There are some basic benchmarks but not sure if those really replicate the real world use cases all that well here. Performance wise I'm mostly concerned with latency when inter-operating with other async libraries.
Hi @jameshilliard thanks for the responses! Ultimately, my goal is to add support for websockets and HTTP proxies in the NATS library. I've put together a PR for websockets here: https://github.com/nats-io/nats.java/pull/426
...but one of the comments that @scottf had in regards to the SocketDataPort changes I made was:
I'd be interested in implementing some sort of factory here to get an implementation based on the protocol, instead of just making this class bigger. May not be practical, just something to consider.
Overall, I agreed with that comment, and since my websockets PR does NOT include support for HTTP proxies, I tried to come up with a better abstraction for the DataPort such that it could be used to layer websockets and/or HTTP proxy support "underneath". That PR is posted here: https://github.com/nats-io/nats.java/pull/494
However, the above PR continues to use the traditional blocking I/O model rather than async I/O as this PR is doing... but when I look at this PR, I notice that NatsConnectionWriter and NatsConnectionReader are ultimately performing blocking calls and continue to occupy 1 thread each, and thus the benefits of async I/O are not achieved.
So, should I put forth the effort to support async I/O model in my DataPort refactor PR here: https://github.com/nats-io/nats.java/pull/494
...or more specifically, do you strongly feel that the proposed NatsChannel abstraction in the above PR should extend AsynchronousByteChannel rather than a regular `ByteChannel?
If so, then I could do the minimal changes to make the NatsChannel async (and thus my refactor PR would be similar to this PR in that NatsConnectionWriter and NatsConnectionReader would continue to convert the async into blocking calls).
However, for this effort to fully pay-off, are you willing to do a follow-up PR to make NatsConnectionWriter and NatsConnectionReader fully async (and not require two threads like it does today)?
I had a conversation with @scottf and it doesn't sound like this library will be moving to async anytime soon. I put together this PR to demonstrate how we could define the low-level NatsChannel as an async class here: https://github.com/nats-io/nats.java/pull/498/files