Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
yschimke opened this issue Jun 13, 2017 · 9 comments
Closed

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

yschimke opened this issue Jun 13, 2017 · 9 comments
Labels
bug help wanted superseded Issue is superseded by another

Comments

@yschimke
Copy link
Member

yschimke commented Jun 13, 2017

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

Currently disabled in settings.gradle

@yschimke
Copy link
Member Author

#310

@robertroeser
Copy link
Member

robertroeser commented Jun 14, 2017

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
Copy link
Member Author

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. :)

@yschimke yschimke changed the title Reenable Aeron Module Aeron Module fails with java.lang.IndexOutOfBoundsException Jun 15, 2017
@robertroeser
Copy link
Member

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
Copy link
Member Author

@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!

@yschimke yschimke changed the title Aeron Module fails with java.lang.IndexOutOfBoundsException Fix or split out rsocket-transport-aeron from rsocket-java Aug 19, 2017
@mboogerd
Copy link

mboogerd commented Dec 1, 2017

+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
Copy link
Member

@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
Copy link

mboogerd commented Dec 4, 2017

@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
Copy link
Member

This was resolved by #538

@rstoyanchev rstoyanchev removed this from the 1.0 milestone Apr 17, 2020
@rstoyanchev rstoyanchev added the superseded Issue is superseded by another label Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted superseded Issue is superseded by another
Projects
None yet
Development

No branches or pull requests

5 participants