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

Upgrade to Netty 4 #30

Closed
jesperpedersen opened this issue Nov 11, 2013 · 12 comments
Closed

Upgrade to Netty 4 #30

jesperpedersen opened this issue Nov 11, 2013 · 12 comments

Comments

@jesperpedersen
Copy link
Contributor

Netty 4 is the active branch in the Netty community, so ideally the project should make use of that version.

Maybe @normanmaurer can help... :)

@kdubb
Copy link
Member

kdubb commented Nov 13, 2013

Is 4 vastly different from 3? Now that I have finished the first pass of SSL support I could look into this but I am no Netty expert.

@normanmaurer
Copy link
Contributor

@kdubb well the API is completely different... So maybe the easiest would be to leave it for me ;) Anyway if you feel brave enough you could check here:
http://netty.io/wiki/new-and-noteworthy.html

@kdubb
Copy link
Member

kdubb commented Nov 13, 2013

Just after I got my head around Netty 3... I'm left in the dust! Haha For now I'll leave it to you if you are interested.

@brettwooldridge
Copy link
Contributor

Yeah, we're looking at a similar migration in our product. Netty 3 -> 4 is a huge change.

@normanmaurer
Copy link
Contributor

@brettwooldridge agree it is a change but we had to do it for performance reasons ;)

@kdubb
Copy link
Member

kdubb commented Dec 2, 2013

@normanmaurer, issues #45 and #46 seem to be related to Netty reporting writes as "succeeded" even when the connection itself has been abandoned. I have searched around and the suggested solution for Netty 3 is a "heartbeat" through IdleStateAwareHandler. Obviously I cannot change the server protocol and I also cannot start sending heartbeat's randomly since if it was just a long running query the server would have garbage.

Honestly I don't quite understand how a write can "succeed" when there is no possible way for an ACK to be sent for it.

Is this something that can be solved by moving to Netty 4? Is there another solution for Netty 3?

@jesperpedersen
Copy link
Contributor Author

@kdubb at least we should bump the minimum requirement to 3.8.0 for now, as there seems to have been many fixes from 3.6.3 to that release

@kdubb
Copy link
Member

kdubb commented Dec 2, 2013

@jesperpedersen Good idea, I shall try that now

@normanmaurer
Copy link
Contributor

@kdubb honestly I would have to look into the code to say anything about #45 and #46. What a succeed write means is that we was able to call java.nio.SocketChannel.write(...) without and exception and the buffer was written completely. That's it, nothing more

@jesperpedersen @kdubb @brettwooldridge I will try to assign some time for next week to move the code base over to Netty 4. So stay tuned...

@kdubb
Copy link
Member

kdubb commented Dec 3, 2013

FYI, I finally discovered my problem (which seems unrelated to Netty itself) and that's that I wasn't dispatching exception from the channel handler.

To top it off, during my testing, I was killing the postmaster process and not the actual backend process servicing the connection... so I wasn't actually killing the correct process at all; this is quite a relief as I couldn't figure out at all how it was still working!

Anyways, I bumped Netty to 3.8 and I believe fixed the "wait indefinitely" on a closed connection bugs.

@brettwooldridge
Copy link
Contributor

Excellent!

@kdubb
Copy link
Member

kdubb commented Dec 3, 2013

@brettwooldridge If you could get back to the version of bitronix that was revealing these bugs and verify they are at least throwing exceptions, that would be awesome.

@kdubb kdubb closed this as completed Feb 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants