JAVA-1241: Updated netty to 4.1.6.Final#763
Conversation
|
Hi @alexbool, thanks for your contribution! In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. Sincerely, |
|
Thank you @alexbool for signing the Contribution License Agreement. Cheers, |
|
Hi, |
|
No, just updating to prevent dll hell with other piece of software that is using |
|
@olim7t |
|
@yvladimirov the java-driver has a 'shaded' configuration (doc) that embeds and shades netty under a different package name in the driver jar. This would allow you to continue using the driver in the case where the version of netty that your application or another dependency has conflicts with the driver's netty dependency version. Would that work in the meantime? |
|
@tolbertam This could work, but in this case we cannot benefit from using native transport, which can lead to performance issues that we have not yet evaluated. |
I haven't myself seen a measured benefit of the epoll module yet for the driver in particular (some discussion here), although it's been quite a while since i've taken some time to look at it and run some comparison tests (and look at the garbage profile in particular). I imagine that you won't notice a difference unless running with very large throughputs (> 100k req/s).
I think this would be a great consideration for the next minor release (i.e. 3.2.x), although i'm not sure when that will be. Really it is just a matter of being accepted and going through our test cycle. I'll give it a run through of our CI this afternoon and get back to you just so we have some initial confidence that this change won't cause any glaring issues. I see the appveyor build (windows) failed an SSL test, likely because it uses the netty-tcnative-openssl module and it's possible that netty 4.1.x needs a newer version of libapr1. I would prefer not to put this in a patch release (i.e. 3.1.3) because we should be limiting the scope of those changes to fixes and minor changes to keep patch releases predictable for users as there may be some backwards compatibility issues with netty 4.1.x and 4.0.x that we aren't aware of. |
|
It looks like CI ran well, the only exception being that we'll need to update our netty-tcnative dependency in our tests and also update our OSGI test to depend on netty 4.1. Would you mind giving me access to update this branch so I can make the appropriate changes? Otherwise I could create another PR. |
|
I think 19fbb55 will fix the test issues if you wouldn't mind cherry-picking it. I'm running things through CI now and if it looks good (if this passes we're good) I think this is good to merge to the 3.x branch (if everyone else is comfortable with it). |
|
Thanks for your help, I cherry-picked you commit. |
|
AppVeyor failed with some strange errors |
|
Indeed it looks like there's still some work that needs to be done on the OSGi tests, i'll fix that up, thanks! |
Agreed, let's target this for 3.2.0. |
|
It looks like the OSGi tests issues were around |
|
@tolbertam Thanks, cherry-picked! |
|
Netty 4.1.8 has been released on Jan 30, we should probably use that instead. |
No description provided.