search

rsocket / rsocket-java

Java implementation of RSocket
http://rsocket.io
Apache License 2.0
2.36k stars 354 forks source link

Fix or split out rsocket-transport-aeron from rsocket-java #309

Closed yschimke closed 6 years ago

yschimke commented 7 years ago

Was "Aeron Module fails with java.lang.IndexOutOfBoundsException"

Currently disabled in settings.gradle

yschimke commented 7 years ago

https://github.com/rsocket/rsocket-java/pull/310

robertroeser commented 7 years ago

In order to use Aeron someone needs to fix the handshaking in the beginning. Basically what needs to happen is the acking that the connection is established needs to happen in the AvailableImageHandler. After the first connection subsequent connections will fail right now.

yschimke commented 7 years ago

I'm going to land this so that the code doesn't bit rot, and I'll try to find some spare time to fix this transport unless @tmontgomery beats me to it. :)

robertroeser commented 7 years ago

Cool -I coded myself to a corner, and I haven't gotten around to fixing it. I should probably take a look at it again... it would be good to have someone else look at this as well.

yschimke commented 7 years ago

@robertroeser @tmontgomery I'm going to split this out into a separate project unless there is an active owner for fixing this. I consider it a blocker for a 1.0 release. Any objections?

On the flipside - if one of you get it working, then I'm happy that we keep it in rsocket-java AND I'll put it as part of the automated TCK tests!

mboogerd commented 7 years ago

+1 to get this issue fixed as I was motivated to use rsocket-java exactly because it has reactive streams on top of Aeron.

For what it is worth, I did attempt for a few hours to provide the requested "help wanted". However, nothing really useful came out of that as I'm still a noob when it comes to the codebase of Aeron, Reactor and ReactiveSocket. I expect other newbies will have similar experiences because there is a good bunch of new concepts to process. That's just some feedback for the core developers of this library; hope it will help in making a choice with regards to the points raised by @yschimke.

robertroeser commented 7 years ago

@mboogerd Hi - we should probably remove it for the time being. I don't have time to get to this right now, and there is probably going to be a change in Aeron that will make it much easier to do, so I'm tempted to just wait for that. I don't know what your use-case is but the TCP version is pretty snappy still, and you can always swap out the transports after the Aeron one is working.

mboogerd commented 7 years ago

@robertroeser taking it out sounds like a good decision; and indeed, I am using the TCP one for the time being. Hopefully I will learn enough in the meantime to be more of use in resolving this issue. Thanks for your feedback!

mostroverkhov commented 6 years ago

This was resolved by #538