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

Introduce new transports driven by java.nio #27260

Closed
9 of 12 tasks
Tim-Brooks opened this issue Nov 3, 2017 · 1 comment
Closed
9 of 12 tasks

Introduce new transports driven by java.nio #27260

Tim-Brooks opened this issue Nov 3, 2017 · 1 comment
Assignees
Labels
:Distributed Coordination/Network Http and internode communication implementations >enhancement Meta Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination.

Comments

@Tim-Brooks
Copy link
Contributor

Tim-Brooks commented Nov 3, 2017

# Description

We are interested in introducing alternative transport options that do not depend on external libraries. From time to time we have had some friction with using netty related to buffer pooling, oom excetions, exception handling, unsafe, etc. We want to look explore if a homegrown transport could potentially meet our needs.

Where we are now

We have a org.elasticsearch.libs.nio library that provides low-level NIO-based network constructs. Everything is built around the NioSelector. This is a single-threaded event loop that handles the operations for a collection of NioChannel. Specific channel behavior based on read, write, or accept events is determined by configuring a ChannelContext.

We have implemented a transport-nio plugin using these facilities. This plugin provides fully functional binary and http transports. Additionally there is a MockNioTransport built on the nio library in the test framework package. This transport is used for all inter-cluster communication in tests.

Work to be completed

  • Byte pooling for NioTransport - we currently reuse bytes for writing. However, we also need to introduce some version for reading. There is an open PR (Implement byte array reusage in nio transport #27051) that includes this work. However, we are still hashing out the details. There is an issue tracking this work (Implement byte array reuse in nio transport #27563).
  • Move base nio classes to a library jar. (Introduce elasticsearch nio library jar #27802)
  • Move NioTransport to a plugin
  • Finish working http implementation (HTTP implementation based on elasticsearch-nio #28898)
  • Byte pooling for nio http transport - this can likely reuse some of the work introduced in Implement byte array reuse in nio transport #27563.
  • Benchmarking - Add to rally benchmarks.
  • Benchmarking - Look into network specific benchmarks
  • Merge AcceptingSelector and SocketSelector. There is not a reason why these need to be separate classes / running threads.
  • Explore using the same network selectors for processing both http and tcp channels. There is no reason we need dedicated network threads for these different transports. Theoretically we might get better resource utilization by sharing the network threads.
  • Experiment with sharing byte pooling. Do we want to use the same page recyclers that we use for various requests? Or do we want to have byte pools dedicated to the network?
    • We will probably need to implement a byte array allocator specific to networking purposes. Our current allocators only support 16KB byte arrays. However, TLS support requires byte arrays at least ~16,900 bytes.
  • Experiment with direct ByteBuffers. Using direct byte buffers prevents some copying when reading and writing to the network. We currently have recyclers that should support pooling direct byte buffers. We should look at performance if we use these for reading / writing opposed to on heap byte arrays.
  • Add bounds to selector queues. There are currently unbounded queues for channels to be closed, writes to be processed, and channels to be opened. Based on this issue (Improve thread pools #18613) maybe we want to bound these? At minimum we probably want major log warns if these queues get too large.
@Tim-Brooks Tim-Brooks added :Distributed Coordination/Network Http and internode communication implementations >enhancement labels Nov 3, 2017
@Tim-Brooks Tim-Brooks self-assigned this Nov 3, 2017
Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this issue Nov 6, 2017
This is related to elastic#27260. Currently `ChannelFactory` is tightly coupled
to classes related to the elasticsearch Tcp binary protocol. This commit
modifies the factory to be able to construct http or other protocol
channels.
Tim-Brooks added a commit that referenced this issue Nov 8, 2017
* Decouple `ChannelFactory` from Tcp classes

This is related to #27260. Currently `ChannelFactory` is tightly coupled
to classes related to the elasticsearch Tcp binary protocol. This commit
modifies the factory to be able to construct http or other protocol
channels.
Tim-Brooks added a commit that referenced this issue Nov 8, 2017
* Decouple `ChannelFactory` from Tcp classes

This is related to #27260. Currently `ChannelFactory` is tightly coupled
to classes related to the elasticsearch Tcp binary protocol. This commit
modifies the factory to be able to construct http or other protocol
channels.
Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this issue Nov 16, 2017
This is related to elastic#27260. In the nio transport work we do not catch or
handle `Throwable`. There are a few places where we have exception
handlers that accept `Throwable`. This commit removes those cases.
Tim-Brooks added a commit that referenced this issue Nov 17, 2017
This is related to #27260. In the nio transport work we do not catch or
handle `Throwable`. There are a few places where we have exception
handlers that accept `Throwable`. This commit removes those cases.
Tim-Brooks added a commit that referenced this issue Nov 17, 2017
This is related to #27260. In the nio transport work we do not catch or
handle `Throwable`. There are a few places where we have exception
handlers that accept `Throwable`. This commit removes those cases.
Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this issue Nov 17, 2017
This is related to elastic#27260. Currently every nio channel has a profile
field. Profile is a concept that only relates to the tcp transport. Http
channels will not have profiles. This commit moves the profile from the
nio channel to the read context. The context is the level that protocol
specific features and logic should live.
Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this issue Nov 17, 2017
This is related to elastic#27260. Currently, every ESSelector keeps track of
all channels that are registered with it. ESSelector is just an
abstraction over a raw java nio selector. The java nio selector already
tracks its own selection keys. This commit removes our tracking and
relies on the java nio selector tracking.
Tim-Brooks added a commit that referenced this issue Nov 17, 2017
This is related to #27260. Currently, every ESSelector keeps track of
all channels that are registered with it. ESSelector is just an
abstraction over a raw java nio selector. The java nio selector already
tracks its own selection keys. This commit removes our tracking and
relies on the java nio selector tracking.
Tim-Brooks added a commit that referenced this issue Nov 17, 2017
This is related to #27260. Currently, every ESSelector keeps track of
all channels that are registered with it. ESSelector is just an
abstraction over a raw java nio selector. The java nio selector already
tracks its own selection keys. This commit removes our tracking and
relies on the java nio selector tracking.
Tim-Brooks added a commit that referenced this issue Nov 20, 2017
This is related to #27260. Currently every nio channel has a profile
field. Profile is a concept that only relates to the tcp transport. Http
channels will not have profiles. This commit moves the profile from the
nio channel to the read context. The context is the level that protocol
specific features and logic should live.
Tim-Brooks added a commit that referenced this issue Nov 20, 2017
This is related to #27260. Currently every nio channel has a profile
field. Profile is a concept that only relates to the tcp transport. Http
channels will not have profiles. This commit moves the profile from the
nio channel to the read context. The context is the level that protocol
specific features and logic should live.
Tim-Brooks added a commit that referenced this issue Nov 22, 2017
This is related to #27260. Currently, basic nio constructs (nio
channels, the channel factories, selector event handlers, etc) implement
logic that is specific to the tcp transport. For example, NioChannel
implements the TcpChannel interface. These nio constructs at some point
will also need to support other protocols (ex: http).

This commit separates the TcpTransport logic from the nio building
blocks.
Tim-Brooks added a commit that referenced this issue Nov 22, 2017
This is related to #27260. Currently, basic nio constructs (nio
channels, the channel factories, selector event handlers, etc) implement
logic that is specific to the tcp transport. For example, NioChannel
implements the TcpChannel interface. These nio constructs at some point
will also need to support other protocols (ex: http).

This commit separates the TcpTransport logic from the nio building
blocks.
Tim-Brooks added a commit that referenced this issue Apr 25, 2019
This is related to #27260. Currently for the SSLDriver we allocate a
dedicated network write buffer and encrypt the data into that buffer one
buffer at a time. This requires constantly switching between encrypting
and flushing. This commit adds a dedicated outbound buffer for SSL
operations that will internally allocate new packet sized buffers as
they are need (for writing encrypted data). This allows us to totally
encrypt an operation before writing it to the network. Eventually it can
be hooked up to buffer recycling.
Tim-Brooks added a commit that referenced this issue Apr 25, 2019
This is related to #27260. Currently for the SSLDriver we allocate a
dedicated network write buffer and encrypt the data into that buffer one
buffer at a time. This requires constantly switching between encrypting
and flushing. This commit adds a dedicated outbound buffer for SSL
operations that will internally allocate new packet sized buffers as
they are need (for writing encrypted data). This allows us to totally
encrypt an operation before writing it to the network. Eventually it can
be hooked up to buffer recycling.
Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this issue Apr 29, 2019
This is related to elastic#27260. Currently for the SSLDriver we allocate a
dedicated network write buffer and encrypt the data into that buffer one
buffer at a time. This requires constantly switching between encrypting
and flushing. This commit adds a dedicated outbound buffer for SSL
operations that will internally allocate new packet sized buffers as
they are need (for writing encrypted data). This allows us to totally
encrypt an operation before writing it to the network. Eventually it can
be hooked up to buffer recycling.
Tim-Brooks added a commit that referenced this issue Apr 29, 2019
This is related to #27260. Currently for the SSLDriver we allocate a
dedicated network write buffer and encrypt the data into that buffer one
buffer at a time. This requires constantly switching between encrypting
and flushing. This commit adds a dedicated outbound buffer for SSL
operations that will internally allocate new packet sized buffers as
they are need (for writing encrypted data). This allows us to totally
encrypt an operation before writing it to the network. Eventually it can
be hooked up to buffer recycling.

This commit also backports the following commit:

Handle WRAP ops during SSL read

It is possible that a WRAP operation can occur while decrypting
handshake data in TLS 1.3. The SSLDriver does not currently handle this
well as it does not have access to the outbound buffer during read call.
This commit moves the buffer into the Driver to fix this issue. Data
wrapped during a read call will be queued for writing after the read
call is complete.
Tim-Brooks added a commit that referenced this issue May 1, 2019
This is related to #27260. Currently there is a setting
http.read_timeout that allows users to define a read timeout for the
http transport. This commit implements support for this functionality
with the transport-nio plugin. The behavior here is that a repeating
task will be scheduled for the interval defined. If there have been
no requests received since the last run and there are no inflight
requests, the channel will be closed.
Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this issue May 1, 2019
This is related to elastic#27260. Currently there is a setting
http.read_timeout that allows users to define a read timeout for the
http transport. This commit implements support for this functionality
with the transport-nio plugin. The behavior here is that a repeating
task will be scheduled for the interval defined. If there have been
no requests received since the last run and there are no inflight
requests, the channel will be closed.
akhil10x5 pushed a commit to akhil10x5/elasticsearch that referenced this issue May 2, 2019
This is related to elastic#27260. Currently for the SSLDriver we allocate a
dedicated network write buffer and encrypt the data into that buffer one
buffer at a time. This requires constantly switching between encrypting
and flushing. This commit adds a dedicated outbound buffer for SSL
operations that will internally allocate new packet sized buffers as
they are need (for writing encrypted data). This allows us to totally
encrypt an operation before writing it to the network. Eventually it can
be hooked up to buffer recycling.
akhil10x5 pushed a commit to akhil10x5/elasticsearch that referenced this issue May 2, 2019
This is related to elastic#27260. Currently there is a setting
http.read_timeout that allows users to define a read timeout for the
http transport. This commit implements support for this functionality
with the transport-nio plugin. The behavior here is that a repeating
task will be scheduled for the interval defined. If there have been
no requests received since the last run and there are no inflight
requests, the channel will be closed.
Tim-Brooks added a commit that referenced this issue May 2, 2019
This is related to #27260. Currently there is a setting
http.read_timeout that allows users to define a read timeout for the
http transport. This commit implements support for this functionality
with the transport-nio plugin. The behavior here is that a repeating
task will be scheduled for the interval defined. If there have been
no requests received since the last run and there are no inflight
requests, the channel will be closed.
Tim-Brooks added a commit that referenced this issue May 2, 2019
This is related to #27260. Currently we have a single read buffer that
is no larger than a single TLS packet. This prevents us from reading
multiple TLS packets in a single socket read call. This commit modifies
our TLS work to support reading similar to the plaintext case. The data
will be copied to a (potentially) recycled TLS packet-sized buffer for
interaction with the SSLEngine.
Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this issue May 5, 2019
This is related to elastic#27260. Currently we have a single read buffer that
is no larger than a single TLS packet. This prevents us from reading
multiple TLS packets in a single socket read call. This commit modifies
our TLS work to support reading similar to the plaintext case. The data
will be copied to a (potentially) recycled TLS packet-sized buffer for
interaction with the SSLEngine.
Tim-Brooks added a commit that referenced this issue May 6, 2019
This is related to #27260. Currently we have a single read buffer that
is no larger than a single TLS packet. This prevents us from reading
multiple TLS packets in a single socket read call. This commit modifies
our TLS work to support reading similar to the plaintext case. The data
will be copied to a (potentially) recycled TLS packet-sized buffer for
interaction with the SSLEngine.
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this issue May 27, 2019
This is related to elastic#27260. Currently for the SSLDriver we allocate a
dedicated network write buffer and encrypt the data into that buffer one
buffer at a time. This requires constantly switching between encrypting
and flushing. This commit adds a dedicated outbound buffer for SSL
operations that will internally allocate new packet sized buffers as
they are need (for writing encrypted data). This allows us to totally
encrypt an operation before writing it to the network. Eventually it can
be hooked up to buffer recycling.
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this issue May 27, 2019
This is related to elastic#27260. Currently there is a setting
http.read_timeout that allows users to define a read timeout for the
http transport. This commit implements support for this functionality
with the transport-nio plugin. The behavior here is that a repeating
task will be scheduled for the interval defined. If there have been
no requests received since the last run and there are no inflight
requests, the channel will be closed.
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this issue May 27, 2019
This is related to elastic#27260. Currently we have a single read buffer that
is no larger than a single TLS packet. This prevents us from reading
multiple TLS packets in a single socket read call. This commit modifies
our TLS work to support reading similar to the plaintext case. The data
will be copied to a (potentially) recycled TLS packet-sized buffer for
interaction with the SSLEngine.
@jakelandis jakelandis added v7.3.0 and removed v7.2.0 labels Jun 17, 2019
@ywelsch ywelsch added 7x and removed v7.3.0 labels Jun 25, 2019
@polyfractal polyfractal removed the 7x label Dec 12, 2019
@rjernst rjernst added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label May 4, 2020
@original-brownbear
Copy link
Member

We stopped this effort #82085

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Network Http and internode communication implementations >enhancement Meta Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination.
Projects
None yet
Development

No branches or pull requests

7 participants