Upgrade to Netty 4.0

[ghost]

I'm currently working on this. Writing the issue here so we could discuss.

Netty 4 is not yet stable, so I won't merge until everything is tested and Netty 4 has a stable release.

[kimchy]

cool, btw, I have been looking into this as well, its quite a bit of work, might request some changes in other parts of ES to properly use the new buffering model (that I still need to fully understand how its implemented).

[ghost]

I haven't looked into how exactly it's implemented, but the API is the same in that regard (other than changing the name from ChannelBuffer to ByteBuf). The ChannelBuffer class was only used in a few pieces, namely to support the HTTP server. Other than that, it's very modular, and swapping out the HTTP servers should be fairly trivial.

What's NOT trivial is that the API is very much streamlined in 4.0, whereas it's not so much in 3.6. I'm spending quite a bit of time trying to decipher the logic in 3.6, but very little time actually translating it to 4.0 once understood.

For your reference, these were the packages impacted:

org.elasticsearch.bulk.udp
org.elasticsearch.common.byte
org.elasticsearch.common.compress
org.elasticsearch.common.compress.lzf
org.elasticsearch.common.netty
org.elasticsearch.common.http.netty
org.elasticsearch.transport.netty

The byte and compress packages were only slightly modified to act as utilities for the other packages. the org.elasticsearch.common.http package was completely generic. I think someone had the foresight to believe that the HTTP functionality would eventually be swapped. org.elasticsearch.common.netty were basically utility functions for the other packages.

The hardest to change will be org.elasticsearch.common.http.netty and org.elasticsearch.transport.netty, which is what I'm up to right now. I won't be able to test until I can resolve the dependencies, so the most time consuming part will be testing and refactoring.

In all, it's a real challenge. But I'm down :)

I'm actually mainly doing this so I can hopefully build in Websocket support. I want to be able to stream searches ;)

-Cris

[spinscale]

Hey,

there was just a post about a major refactoring from one netty 4 CR to another CR version. See http://netty.io/news/2013/06/18/4-0-0-CR5.html

Just wanted to ensure, that one cannot be totally confident, that this is the last breaking netty 4 change by the netty team (which I like personally, as performance is a first class citizen here).

--Alex

[ghost]

Thanks for the heads up! I've been on their IRC, but I don't think I've managed to talk with anyone on their team (or at least I'm not sure :) ). Right now, I'm working off of their current branch, so this is actually 4.0.0-CR7-SNAPSHOT. I am expecting it to change, but I figured now would be a good time to at least start. It's drastic enough from 3.6 that I think going from 4.0.0-CR7 to release would be easier.

Also I want this done for my own uses sooner than waiting :)

[kimchy]

The way I saw it, to properly use the new netty 4 features, we will need to make use of the new buffering mechanisms to reduce buffer copies. Currently, we heavily rely on how buffers are handled in netty 3 in the IO reader and writer threads, which we will probably need to properly understand to use with netty 4. I think (haven't looked closely) that we will need to change things like how REST actions serialize data to make sure we reduce buffer copies.

[ghost]

I've just been replacing the data types with the Netty 4 equivalents, but I'm at the point where I have to rewrite the pipelining. I'll go back and take a look at buffering, because I didn't exactly take a close look under the hood.

[icedfish]

Hi all,
Netty 4.0 Stable has been released :
https://github.com/netty/netty/releases/tag/netty-4.0.0.Final

[ghost]

I'll take a look again soon.

[GrantGochnauer]

+1 - http://www.infoq.com/news/2013/11/netty4-twitter

[kimchy]

I had another look, and I am not too happy with some of the netty 4 behavior.Note, the behavior people see (less GC) happens with "stock" netty, we do quite a few tricky to reduce the buffer copies already. The migration will require quite a bit of work in order to maintain those optimizations... (we will do it anyhow). Also need to find the time to raise some of the issues with netty...

[GrantGochnauer]

Thanks for the update Shay!

[normanmaurer]

@kimchy what "issues" you are talking about? Maybe I can give you some pointers.

[kimchy]

@normanmaurer I will start respective questions properly on the netty mailing list.

[normanmaurer]

@kimchy alright..

[kimchy]

@normanmaurer cool, we are focused on getting a new version of ES out, so not focusing on netty 4 now, especially with the problematic aspects I saw there and the fact that it will be a bigger change than expected. And trust me, the "issues" there should not have quotation marks as in your comment, we use it in quite an advance manner make sure we don't allocate buffers needlessly, and the refactoring done in netty 4 is, well, tricky.... (admittedly, it does help naive handlers).

[hhoffstaette]

@kimchy Mind if I take this? I'll gladly collect any concerns (sort of doing it already) and write up/research/do the work. I'm also now on the Netty list.

[normanmaurer]

Feel free to ping me if any questions come up

Am 14.02.2014 um 23:16 schrieb Holger Hoffstätte notifications@github.com:

@kimchy Mind if I take this? I'll gladly collect any concerns (sort of doing it already) and write up/research/do the work.

—
Reply to this email directly or view it on GitHub.

[jabley]

Ohai.

https://github.com/jabley/elasticsearch/compare/enhancement/netty_4_upgrade

I think this is quite close, I'll try to get the 5 test failures passing soon.

Tests with failures (first 3 out of 5):
  - org.elasticsearch.options.jsonp.JsonpOptionDisabledTest.testThatJSONPisDisabled
  - org.elasticsearch.options.jsonp.JsonpOptionEnabledTest.testThatJSONPisEnabled
  - org.elasticsearch.transport.netty.SimpleNettyTransportTests.testVersion_from0to0

Those tests pass in isolation, so I'll have to dig a bit.

[jabley]

mvn test -Dtests.seed=2025BC3DED852C8E -Dtests.class=org.elasticsearch.cluster.ClusterServiceTests -Dtests.prefix=tests -Dfile.encoding=UTF-8 -Duser.timezone=Europe/London -Des.logger.level=INFO -Des.node.local=1 -Dtests.heap.size=512m -Dtests.bwc.path=/Users/jabley/git/elasticsearch/backwards -Dtests.processors=8

that is failing on my branch.

Tests with failures:
  - org.elasticsearch.cluster.ClusterServiceTests.testListenerCallbacks
  - org.elasticsearch.cluster.ClusterServiceTests.testPendingUpdateTask

Those tests pass in isolation when I used -Dtests.method="testPendingUpdateTask". I haven't figured it out yet; I'm presuming there's some shared state that I've not spotted.

[jasontedor]

Feel free to ping me if any questions come up

@normanmaurer As Netty 3.x is deprecated but is still currently maintained, do the Netty Project maintainers have a rough timeframe for when it will no longer be the case that it is maintained?

[philnate]

Any news on when this will be done? It's causing some trouble with newer nettys, which use the same artefact names, but different packages. Could at least netty be shaded like the other dependencies are?

[jasontedor]

Any news on when this will be done?

Not in the near future.

It's causing some trouble with newer nettys, which use the same artefact names, but different packages.

What's the issue, since they are in different packages (org.jboss.netty versus io.netty)?

Could at least netty be shaded like the other dependencies are?

We don't shade dependencies after the 1.x series, and are not going to make a change like that for the 1.x series.

[normanmaurer]

Just to be clear netty 4.x and 3.x use different groupId's and different packages so it should be no problem to use both versions in the same app.

[philnate]

Yeah, I got lost in dependencies. Somehow some old org.jetty.netty got pulled in which didn't fit elasticsearchs needs. I think what got me confused is that mvnrepository tells that ES 2.2 uses io.netty:netty (which in fact has org.jetty.netty packages) http://mvnrepository.com/artifact/org.elasticsearch/elasticsearch/2.2.0, this package would conflict with never nettys. I've pulled in io.netty:netty:3.x and io.netty:netty-all:4.y and it worked out.

[jasontedor]

Relates netty/netty#5361 which is a proposal to discontinue support for Netty 3.x.