Create nio-transport plugin for NioTransport #27949
Conversation
Tim-Brooks
added
:Distributed Coordination/Network
rjernst
left a comment
rjernst
left a comment
There was a problem hiding this comment.
I reviewed the gradle/plugin side.
plugins/transport-nio/build.gradle
Outdated
| compileTestJava.options.compilerArgs << "-Xlint:-rawtypes,-unchecked" | ||
|
|
||
| dependencies { | ||
| compile project(path: ':libs:elasticsearch-nio', configuration: 'runtime') |
There was a problem hiding this comment.
Is the plan to publish the nio jar (since it is used by the test framework, and may be used by the rest client in the future)? If so, the pattern we have used in the rest of the build is to setup a project substitution, and use maven coordinates here.
| @Override | ||
| public Settings additionalSettings() { | ||
| final Settings.Builder settingsBuilder = Settings.builder(); | ||
| settingsBuilder.put(NetworkModule.TRANSPORT_TYPE_KEY, NIO_TRANSPORT_NAME); |
There was a problem hiding this comment.
For the cloud plugins, we decided against "auto configuring" like this, since if there are multiple plugins trying to set themselves as an implementation, the last one loaded wins.
|
|
||
| @FunctionalInterface | ||
| interface ReadConsumer { | ||
|
|
| public class NioNotEnabledBootstrapCheck implements BootstrapCheck { | ||
|
|
||
| @Override | ||
| public BootstrapCheckResult check(BootstrapContext context) { |
| serverAcceptedChannel((TcpChannel) channel); | ||
| } | ||
|
|
||
| private static BytesReference toBytesReference(InboundChannelBuffer channelBuffer) { |
There was a problem hiding this comment.
can we add a utilitiy that is shared for this somehow, does it make sense?
| return new CompositeBytesReference(references); | ||
| } | ||
|
|
||
| private class MockTcpChannelFactory extends ChannelFactory<MockServerChannel, MockSocketChannel> { |
There was a problem hiding this comment.
I wonder if we can extract this into a common place. maybe we just have utility methods there that we call. but I would love to reduce the footprint of this class here. I am ok with delegating but it should be mostly shared. We might be able to pass in factories for MockSocketChannel?
|
@s1monw I made a number of changes based on your comments. I pulled quite a bit of duplicate decoding work into |
s1monw
left a comment
s1monw
left a comment
There was a problem hiding this comment.
this looks great! thanks for the extra iteration
| /** | ||
| * Returns an array of byte buffers from the given BytesReference. | ||
| */ | ||
| public static BytesReference fromByteBuffers(ByteBuffer[] buffers) { |
* master: (25 commits) Remove Gradle cheatsheet Fix reproduction info to point to Gradle wrapper Update platforms tests to use Gradle wrapper Update testing docs to reflect Gradle wrapper Painless: Modify Loader to Load Classes Directly from Definition (elastic#28088) Update contributing docs to use the Gradle wrapper Create nio-transport plugin for NioTransport (elastic#27949) test: replaced try-catch statements with expectThrows(...) Add getWarmer and getTranslog method to NodeIndicesStats (elastic#28092) fix doc mistake Added ASN support for Ingest GeoIP plugin. Fix global aggregation that requires breadth first and scores (elastic#27942) Introduce Gradle wrapper Ignore GIT_COMMIT when calculating commit hash Re-enable bwc tests after elastic#27881 was backported Set the elasticsearch-nio codebase for tests (elastic#28067) Bump compat version for local depdendent test to 6.2.0 Pass `java.locale.providers=COMPAT` to Java 9 onwards (elastic#28080) Allow shrinking of indices from a previous major (elastic#28076) Remove deprecated exceptions (elastic#28059) ...
* master: Remove Gradle cheatsheet Fix reproduction info to point to Gradle wrapper Update platforms tests to use Gradle wrapper Update testing docs to reflect Gradle wrapper Painless: Modify Loader to Load Classes Directly from Definition (elastic#28088) Update contributing docs to use the Gradle wrapper Create nio-transport plugin for NioTransport (elastic#27949)
* master: Remove out-of-date projectile file Remove Gradle cheatsheet Fix reproduction info to point to Gradle wrapper Update platforms tests to use Gradle wrapper Update testing docs to reflect Gradle wrapper Painless: Modify Loader to Load Classes Directly from Definition (#28088) Update contributing docs to use the Gradle wrapper Create nio-transport plugin for NioTransport (#27949) test: replaced try-catch statements with expectThrows(...) Add getWarmer and getTranslog method to NodeIndicesStats (#28092) fix doc mistake Added ASN support for Ingest GeoIP plugin. Fix global aggregation that requires breadth first and scores (#27942)
4 participants
This is related to #27260. This commit moves the
NioTransportfrom:test:framework to a new nio-transport plugin. Additionally, supporting
tcp decoding classes are moved to this plugin. Generic byte reading and
writing contexts are moved to the nio library.
Additionally, this commit adds a basic
MockNioTransportto:test:framework that is a
TcpTransportimplementation for testing thatis driven by nio.