Use Netty4 Transport in Internal Cluster Tests #82088
Conversation
Removing the mock nio transport and replacing its usage with the netty transport to make tests with a more realistic transport implementation. This way improves the real world coverage for the Netty transport, makes our tests more realistic and saves lots of code. In particular, coverage on the rather complicated throttling/chunking in the netty message handler is really ice to have. The downside of this change is that we lose the slow transport thread warnings that the mock transport outputs. This isn't a big deal these days in my opinion as we have slow logging in other places now that makes up for this (we didn't when initially adding the slow logging) and that contains far more detailed information on what exactly was slow. Other than that, the mock transport does not come with any features we don't also have in the Netty transport at this point.
original-brownbear
added
>test
elasticmachine
added
Team:Delivery
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
Pinging @elastic/es-delivery (Team:Delivery) |
| Releasables.close(() -> { | ||
| if (channel.isOpen()) { | ||
| try { | ||
| channel.config().setOption(ChannelOption.SO_LINGER, 0); |
There was a problem hiding this comment.
I believe this brings back the logging mentioned in #82360 - I think we need to drop Connection reset logs to DEBUG if transport.netty.rst_on_close is set.
There was a problem hiding this comment.
Ah right It does ... looking into a way that adjusts the level (slightly tricky to find a nice way) ...
There was a problem hiding this comment.
Alright worked around this now by adding a setting for all transports (though only Netty does pick it up). Not super happy with the approach but it works. We could also go with just a static boolean via a property but I tend to rather make things work out of the box in my idea :D
|
Jenkins run elasticsearch-ci/part-1 (unrelated + known) |
|
@DaveCTurner @henningandersen ping if you have a second, this needs a little broader consensus :) |
server/src/main/java/org/elasticsearch/common/transport/NetworkExceptionHelper.java
Show resolved
Hide resolved
test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java
Outdated
Show resolved
Hide resolved
…kExceptionHelper.java Co-authored-by: David Turner <[email protected]>
…stCluster.java Co-authored-by: David Turner <[email protected]>
|
Thanks everyone! |
💔 Backport failed
You can use sqren/backport to manually backport by running |
6 participants
Removing the mock nio transport and replacing its usage with the Netty transport to make tests
with a more realistic transport implementation. This way improves the real world coverage for
the Netty transport, makes our tests more realistic and saves lots of code.
In particular, coverage on the rather complicated throttling/chunking in the Netty message handler
is really ice to have.
The downside of this change is that we lose the slow transport thread warnings that the mock transport
outputs. This isn't a big deal these days in my opinion as we have slow logging in other places
now that makes up for this (we didn't when initially adding the slow logging) and that contains
far more detailed information on what exactly was slow.
Other than that, the mock transport does not come with any features we don't also have in the Netty
transport at this point.