Commit Graph

99 Commits

Author SHA1 Message Date
Jay Modi 01502893eb HTTP transport stashes the ThreadContext instead of the RestController (#23456)
Previously, the RestController would stash the context prior to copying headers. However, there could be deprecation
log messages logged and in turn warning headers being added to the context prior to the stashing of the context. These
headers in the context would then be removed from the request and also leaked back into the calling thread's context.

This change moves the stashing of the context to the HttpTransport so that the network threads' context isn't
accidentally populated with warning headers and to ensure the headers added early on in the RestController are not
excluded from the response.
2017-03-02 14:44:01 -05:00
javanna 9a2dba3036 [TEST] add support for binary responses to REST tests infra 2017-02-27 12:27:03 +01:00
Ryan Ernst 9df95def90 Build: Remove extra copies of netty license (#23361)
The dependencyLicenses check has the ability to map multiple jar files
to the same license file. However, netty was not taking advantage of
this, and had duplicate copies of its license/notice files for each jar.
This commit reduces the copies to one and uses the mapping feature.
2017-02-24 14:40:07 -08:00
Jason Tedor f85a7aed37 Keep the pipeline handler queue small initially
This commit sets the intial size of the pipeline handler queue small to
prevent waste if pipelined requests are never sent. Since the queue will
grow quickly if pipeline requests are indeed set, this should not be
problematic.

Relates #23335
2017-02-23 14:17:46 -05:00
sabi0 09b3c7f270 Do not create String instances in 'Strings' methods accepting StringBuilder (#22907) 2017-02-23 10:57:34 -08:00
Jason Tedor 3e69c38dbd Respect promises on pipelined responses
When pipelined responses are sent to the pipeline handler for writing,
they are not necessarily written immediately. They must be held in a
priority queue until all responses preceding the given response are
written. This means that when write is invoked on the handler, the
promise that is attached to the write invocation will not necessarily be
the promise associated with the responses that are written while the
queue is drained. To address this, the promise associated with a
pipelined response must be held with the response and then used when the
channel context is actually written to. This was introduced when
ensuring that the releasing promise is always chained through on write
calls lest the releasing promise never be invoked. This leads to many
failing test cases, so no new test cases are needed here.

Relates #23317
2017-02-23 09:32:43 -05:00
Jason Tedor 6ca90a61a6 Relocate a comment in HttpPipeliningHandler
This commit moves a comment in HttpPipeliningHandler as it makes more
sense for this comment to be where the field that it is explaining is
declared.
2017-02-22 20:51:18 -05:00
Jason Tedor 30f723d2b0 Add comments to HttpPipeliningHandler
This commit adds some comments explaining the design of
HttpPipeliningHandler.
2017-02-22 20:47:34 -05:00
Jason Tedor 708d11f54a Ensure that releasing listener is called
When sending a response to a client, we attach a releasing listener to
the channel promise. If the client disappears before the response is
sent, the releasing listener was never notified. The reason the
listeners were never notified was due to a mistaken invocation of write
and flush on the channel which has two overrides: one that takes an
existing promise, and one that does not and instead creates a new
promise. When the client disappears, it is this latter promise that is
notified, which does not contain the releasing listener. This commit
addreses this issue by invoking the override that passes our channel
promise through.

Relates #23310
2017-02-22 13:54:17 -05:00
Daniel Mitterdorfer 0744a00001 Set network receive predictor size to 32kb (#23284)
Previously we calculated Netty' receive predictor size for HTTP and transport
traffic based on available memory and worker nodes. This resulted in a receive
predictor size between 64kb and 512kb. In our benchmarks this leads to increased
GC pressure.

With this commit we set Netty's receive predictor size to 32kb. This value is in
a sweet spot between heap memory waste (-> GC pressure) and effect on request
metrics (achieved throughput and latency numbers).

Closes #23185
2017-02-21 14:45:33 +01:00
Jay Modi b234644035 Enforce Content-Type requirement on the rest layer and remove deprecated methods (#23146)
This commit enforces the requirement of Content-Type for the REST layer and removes the deprecated methods in transport
requests and their usages.

While doing this, it turns out that there are many places where *Entity classes are used from the apache http client
libraries and many of these usages did not specify the content type. The methods that do not specify a content type
explicitly have been added to forbidden apis to prevent more of these from entering our code base.

Relates #19388
2017-02-17 14:45:41 -05:00
Jason Tedor 0a5917d182 Fix get HEAD requests
Get HEAD requests incorrectly return a content-length header of 0. This
commit addresses this by removing the special handling for get HEAD
requests, and just relying on the general mechanism that exists for
handling HEAD requests in the REST layer.

Relates #23186
2017-02-15 13:07:29 -05:00
Ryan Ernst 79a1629f74 Fix line length 2017-02-14 21:23:21 -08:00
Jason Tedor 9e80e290d6 Add failing tests for expect header violations
This commit adds unit tests for two cases where Elasticsearch violates
expect header handling. These tests are marked as awaits fix.

Relates #23173
2017-02-14 19:24:22 -05:00
Jason Tedor 673754b1d5 Fix get source HEAD requests
Get source HEAD requests incorrectly return a content-length header of
0. This commit addresses this by removing the special handling for get
source HEAD requests, and just relying on the general mechanism that
exists for handling HEAD requests in the REST layer.

Relates #23151
2017-02-14 16:37:22 -05:00
Jason Tedor 5343b87502 Handle bad HTTP requests
When Netty decodes a bad HTTP request, it marks the decoder result on
the HTTP request as a failure, and reroutes the request to GET
/bad-request. This either leads to puzzling responses when a bad request
is sent to Elasticsearch (if an index named "bad-request" does not exist
then it produces an index not found exception and otherwise responds
with the index settings for the index named "bad-request"). This commit
addresses this by inspecting the decoder result on the HTTP request and
dispatching the request to a bad request handler preserving the initial
cause of the bad request and providing an error message to the client.

Relates #23153
2017-02-13 17:39:25 -05:00
Jay Modi 61e383813d Make the version of the remote node accessible on a transport channel (#23019)
This commit adds a new method to the TransportChannel that provides access to the version of the
remote node that the response is being sent on and that the request came from. This is helpful
for serialization of data attached as headers.
2017-02-13 15:15:57 -05:00
Jason Tedor 0f21ed5b70 Fix template HEAD requests
Template HEAD requests incorrectly return a content-length header of
0. This commit addresses this by removing the special handling for
template HEAD requests, and just relying on the general mechanism that
exists for handling HEAD requests in the REST layer.

Relates #23130
2017-02-11 18:30:16 -05:00
Jason Tedor a6158398dd Fix index HEAD requests
Index HEAD requests incorrectly return a content-length header of
0. This commit addresses this by removing the special handling for index
HEAD requests, and just relying on the general mechanism that exists for
handling HEAD requests in the REST layer.

Relates #23112
2017-02-10 09:44:01 -05:00
Jason Tedor 7ac44656df Fix alias HEAD requests
Alias HEAD requests incorrectly return a content-length header of
0. This commit addresses this by removing the special handling for alias
HEAD requests, and just relying on the general mechanism that exists for
handling HEAD requests in the REST layer.

Relates #23094
2017-02-10 09:19:35 -05:00
Tim Brooks a331405aff Isolated SocketPermissions to Netty (#23057)
Netty 4.1.8 wraps connect and accept operations in doPrivileged blocks.
This means that we not need to give permissions to the entire transport
module. Additionally this commit deletes the privileged socket channel
and privileged server socket chanel.
2017-02-09 10:00:25 -06:00
Tim Brooks 735e5b1983 Upgrade to Netty 4.1.8 (#23055)
This commit upgrades the Netty dependency to version 4.1.8.Final.
2017-02-08 11:44:36 -06:00
Boaz Leskes ba06c14a97 TransportService.connectToNode should validate remote node ID (#22828)
#22194 gave us the ability to open low level temporary connections to remote node based on their address. With this use case out of the way, actual full blown connections should validate the node on the other side, making sure we speak to who we think we speak to. This helps in case where multiple nodes are started on the same host and a quick node restart causes them to swap addresses, which in turn can cause confusion down the road.
2017-02-07 22:11:32 +02:00
Tim Brooks f70188ac58 Remove connect SocketPermissions from core (#22797)
This is related to #22116. Core no longer needs `SocketPermission`
`connect`.

This permission is relegated to these modules/plugins:
- transport-netty4 module
- reindex module
- repository-url module
- discovery-azure-classic plugin
- discovery-ec2 plugin
- discovery-gce plugin
- repository-azure plugin
- repository-gcs plugin
- repository-hdfs plugin
- repository-s3 plugin

And for tests:
- mocksocket jar
- rest client
- httpcore-nio jar
- httpasyncclient jar
2017-02-03 09:39:56 -06:00
Christoph Büscher c33f894846 Fixing compilation problem in Eclipse (#22956) 2017-02-03 16:16:51 +01:00
Jason Tedor 9a0b216c36 Upgrade checkstyle to version 7.5
This commit upgrades the checkstyle configuration from version 5.9 to
version 7.5, the latest version as of today. The main enhancement
obtained via this upgrade is better detection of redundant modifiers.

Relates #22960
2017-02-03 09:46:44 -05:00
Jay Modi 7520a107be Optionally require a valid content type for all rest requests with content (#22691)
This change adds a strict mode for xcontent parsing on the rest layer. The strict mode will be off by default for 5.x and in a separate commit will be enabled by default for 6.0. The strict mode, which can be enabled by setting `http.content_type.required: true` in 5.x, will require that all incoming rest requests have a valid and supported content type header before the request is dispatched. In the non-strict mode, the Content-Type header will be inspected and if it is not present or not valid, we will continue with auto detection of content like we have done previously.

The content type header is parsed to the matching XContentType value with the only exception being for plain text requests. This value is then passed on with the content bytes so that we can reduce the number of places where we need to auto-detect the content type.

As part of this, many transport requests and builders were updated to provide methods that
accepted the XContentType along with the bytes and the methods that would rely on auto-detection have been deprecated.

In the non-strict mode, deprecation warnings are issued whenever a request with body doesn't provide the Content-Type header.

See #19388
2017-02-02 14:07:13 -05:00
Tim Brooks a4ac29c005 Add single static instance of SpecialPermission (#22726)
This commit adds a SpecialPermission constant and uses that constant
opposed to introducing new instances everywhere.

Additionally, this commit introduces a single static method to check that
the current code has permission. This avoids all the duplicated access
blocks that exist currently.
2017-01-21 12:03:52 -06:00
Tim Brooks bc16162d21 Remove accept SocketPermissions from core (#22622)
This is related to #22116. Core no longer needs SocketPermission 
accept. This permission is relegated to the transport-netty4 module 
and (for tests) to the mocksocket jar.
2017-01-20 09:27:45 -06:00
Ali Beyad e2977889b8 Allow comma delimited array settings to have a space after each entry (#22591)
Previously, certain settings that could take multiple comma delimited
values would pick up incorrect values for all entries but the first if
each comma separated value was followed by a whitespace character.  For
example, the multi-value "A,B,C" would be correctly parsed as
["A", "B", "C"] but the multi-value "A, B, C" would be incorrectly parsed
as ["A", " B", " C"].

This commit allows a comma separated list to have whitespace characters
after each entry.  The specific settings that were affected by this are:

  cluster.routing.allocation.awareness.attributes
  index.routing.allocation.require.*
  index.routing.allocation.include.*
  index.routing.allocation.exclude.*
  cluster.routing.allocation.require.*
  cluster.routing.allocation.include.*
  cluster.routing.allocation.exclude.*
  http.cors.allow-methods
  http.cors.allow-headers

For the allocation filtering related settings, this commit also provides
validation of each specified entry if the filtering is done by _ip,
_host_ip, or _publish_ip, to ensure that each entry is a valid IP
address.

Closes #22297
2017-01-17 08:51:04 -06:00
Tim Brooks 16a76d9bc0 Remove blocking TCP clients and servers (#22639)
This commit removes the option to use the blocking variants of the TCP
transport server, TCP transport client, or http server.
2017-01-16 18:38:51 -06:00
Simon Willnauer f30b1f82ee Remove HttpServer and HttpServerAdapter in favor of a simple dispatch method (#22636)
Today we have quite some abstractions that are essentially providing a simple
dispatch method to the plugins defining a `HttpServerTransport`. This commit
removes `HttpServer` and `HttpServerAdaptor` and introduces a simple `Dispatcher` functional
interface that delegate to `RestController` by default.

Relates to #18482
2017-01-16 21:06:08 +01:00
Tim Brooks f4270f9914 Wrap netty accept/connect ops with doPrivileged (#22572)
This is related to #22116. netty channels require socket `connect` and
`accept` privileges. Netty does not currently wrap these operations
with `doPrivileged` blocks. These changes extend the netty channels
and wrap calls to the relevant super methods in doPrivileged blocks.
2017-01-13 14:27:09 -06:00
Jason Tedor 126efea56c Upgrade to Netty 4.1.7
This commit upgrades the Netty dependency to version 4.1.7.Final,
picking up some important bug fixes.

Relates #22587
2017-01-12 10:58:21 -05:00
Tim B be22a250b6 Replace Socket, ServerSocket, and HttpServer usages in tests with mocksocket versions (#22287)
This integrates the mocksocket jar with elasticsearch tests. Mocksocket wraps actions requiring SocketPermissions in doPrivilege blocks. This will eventually allow SocketPermissions to be assigned to the mocksocket jar opposed to the entire elasticsearch codebase.
2017-01-04 14:38:51 -06:00
Jason Tedor 96ba45e310 Fix stale comment in Netty4Utils
We previously named the thread using a frame from the stack trace, but
this was removed to simplify the code here. However, the comment
explaining this was left behind and this commit cleans that up.
2017-01-03 08:15:57 -05:00
Nik Everett f5f2149ff2 Remove much ceremony from parsing client yaml test suites (#22311)
* Remove a checked exception, replacing it with `ParsingException`.
* Remove all Parser classes for the yaml sections, replacing them with static methods.
* Remove `ClientYamlTestFragmentParser`. Isn't used any more.
* Remove `ClientYamlTestSuiteParseContext`, replacing it with some static utility methods.

I did not rewrite the parsers using `ObjectParser` because I don't think it is worth it right now.
2016-12-22 11:00:34 -05:00
Nik Everett a04dcfb95b Introduce XContentParser#namedObject (#22003)
Introduces `XContentParser#namedObject which works a little like
`StreamInput#readNamedWriteable`: on startup components register
parsers under names and a superclass. At runtime we look up the
parser and call it to parse the object.

Right now the parsers take a context object they use to help with
the parsing but I hope to be able to eliminate the need for this
context as most what it is used for at this point is to move
around parser registries which should be replaced by this method
eventually. I make no effort to do so in this PR because it is
big enough already. This is meant to the a start down a road that
allows us to remove classes like `QueryParseContext`,
`AggregatorParsers`, `IndicesQueriesRegistry`, and
`ParseFieldRegistry`.

The goal here is to reduce the amount of plumbing required to
allow parsing pluggable things. With this you don't have to pass
registries all over the place. Instead you must pass a super
registry to fewer places and use it to wrap the reader. This is
the same tradeoff that we use for NamedWriteable and it allows
much, much simpler binary serialization. We think we want that
same thing for xcontent serialization.

The only parsing actually converted to this method is parsing
`ScoreFunctions` inside of `FunctionScoreQuery`. I chose this
because it is relatively self contained.
2016-12-20 11:05:24 -05:00
Simon Willnauer ccfeac8dd5 Remove `doHandshake` test-only settings from TcpTransport (#22241)
In #22094 we introduce a test-only setting to simulate transport
impls that don't support handshakes. This commit implements the same logic
without a setting.
2016-12-18 09:26:53 +01:00
Jason Tedor df43c268da Eagerly initialize Netty 4
Today we initialize Netty in a static initializer. We trigger this
method via static initializers from Netty-related classes, but we can
trigger this method earlier than we do to ensure that Netty is
initialized how we want it to be.
2016-12-15 13:24:47 -05:00
Simon Willnauer 80d6539e9c Handle connection close / reset events gracefully during handshake (#22178)
Low level handshake code doesn't handle situations gracefully if the connection
is concurrently closed or reset by peer. This commit adds the relevant code to
fail the handshake if the connection is closed.
2016-12-14 23:04:14 +01:00
Simon Willnauer 7a9b667e98 Introduce a low level protocol handshake (#22094)
Today we rely on the version that the API user passes in together with the DiscoveryNode. This commit introduces a low level handshake where nodes exchange their version to be used with the transport protocol that is executed every time a connection to a node is established. This, on the one hand allows to change the wire protocol based on the version we are talking to even without a full cluster restart. Today we would need to carry on a BWC layer across major versions but with a handshake we can rely on the fact that the latest version of the previous minor executes a handshake and uses the latest protocol version across all communication with the N+1 version nodes.

This change is yet fully backwards compatible, a followup PR will remove the BWC in 6.0 once this has been back-ported to the 5.x branch
2016-12-13 21:06:23 +01:00
Simon Willnauer 01d67e09b9 Detach handshake from connect to node (#22037)
Today we connect and publish the nodes connection before we execute a
handshake with the node we connect to. In the case of connecting to a node
that won't pass the handshake this connection is already `published` and other
code paths can use it. This commit detaches the connection and the publish of the
connection such that `TransportService` can do a handshake before actually connect
and publish the connection.
2016-12-10 10:03:26 +01:00
Simon Willnauer 842e00c689 [TEST] Add back skip of external clusters 2016-12-02 11:53:33 +01:00
Simon Willnauer 572b4c3e72 Port assert from 5.x to master
I added an assertion to Netty4/Netty3Transport in 5.x that is not in
master yet. This commit port the assert to ensure we consumed all connection
in `connectToChannels`
2016-12-02 10:34:33 +01:00
Simon Willnauer adf9bd90a4 Remove legacy BWC test infrastructure and tests (#21915)
We don't use the test infra nor do we run the tests. They might all be
entirely out of date. We also have a different BWC test infra in-place.
This change removes all of the legacy infra.
2016-12-02 08:06:20 +01:00
Simon Willnauer 155de53fe3 Add a connect timeout to the ConnectionProfile to allow per node connect timeouts (#21847)
Timeouts are global today across all connections this commit allows to specify
a connection timeout per node such that depending on the context connections can
be established with different timeouts.

Relates to #19719
2016-12-01 15:39:49 +01:00
Simon Willnauer dd5256c324 Reduce number of connections per node depending on the nodes role (#21849)
We currently treat every node equally when we establish connections to a node.
Yet, if we are not master eligible or can't hold any data there is no point in creating
a dedicated connection for sending the cluster state or running remote recoveries respectively.
The usage of STATE and RECOVERY connections on non-master and/or non-data nodes will result in an IllegalStateException.
2016-12-01 08:00:48 +01:00
Adrien Grand 6231009a8f Remove 2.x backward compatibility of mappings. (#21670)
For the record, I also had to remove the geo-hash cell and geo-distance range
queries to make the code compile. These queries already throw an exception in
all cases with 5.x indices, so that does not hurt any more.

I also had to rename all 2.x bwc indices from `index-${version}` to
`unsupported-${version}` to make `OldIndexBackwardCompatibilityIT`
happy.
2016-11-30 13:34:46 +01:00
Simon Willnauer f5ff69fabe Remove connectToNodeLight and replace it with a connection profile (#21799)
The Transport#connectToNodeLight concepts is confusing and not very flexible.
neither really testable on a unittest level. This commit cleans up the code used
to connect to nodes and simplifies transport implementations to share more code.
This also allows to connect to nodes with custom profiles if needed, for instance
future improvements can be added to connect to/from nodes that are non-data nodes without
dedicated bulks and recovery connections.
2016-11-29 09:35:07 +01:00