This updates the gradle wrapper to 8.12
We addressed deprecation warnings due to the update that includes:
- Fix change in TestOutputEvent api
- Fix deprecation in groovy syntax
- Use latest ospackage plugin containing our fix
- Remove project usages at execution time
- Fix deprecated project references in repository-old-versions
(cherry picked from commit ba61f8c7f7)
Deprecate to, from, include_lower, include_upper range query params.
These params have been removed from our documentation in v. 0.90.4 (d6ecdecc19),
but did not got through deprecation cycle.
These params to be removed in v9.0.
Related to #81276Closes#48538
With the transport client gone, lots of these constructors have become
unused. Removing this dead code also allows making a lot of fields final
as an added bonus.
This change adds additional plumbing to pipe through the available cluster features into
SearchSourceBuilder. A number of different APIs use SearchSourceBuilder so they had to make this
available through their parsers as well often through ParserContext. This change is largely mechanical
passing a Predicate into existing REST actions to check for feature availability.
Note that this change was pulled mostly from this PR (#105040).
* Use String.replace() instead of replaceAll() for non-regexp replacements
When arguments do not make use of regexp features replace() is a more efficient option, especially the char-variant.
Loads of code here that is only used in tests and one duplicate unused
class that was only used as an indirection to parsing the
`AsyncSearchResponse`. Moved what I could easily move via automated
refactoring to `SearchResponseUtils` in tests and removed the duplicate
now unused class from the client codebase.
A predicate to check whether the cluster supports a feature is available
to rest handlers defined in server. This commit adds that predicate to
plugins defining rest handlers as well.
`ActionType` represents an action which runs on the local node, there's
no need for implementations to define a `Reader<Response>`. This commit
removes the unused constructor argument.
Final step in #102030 ... actually makes `SearchHit` read a releasable bytes reference.
Does still fallback to copying to unrolled buffers here and there which can be removed in follow-ups where it's worth the effort (aggs being the most important one probably).
Hard to create very reliable benchmarks for this because all our macro-benchmarks are quite noisy. Running http logs and PMC though, there's a statistically significant reduction in GC and reduced tail latencies in most benchmarks.
The overhead for ref-counting these bytes isn't visible in profiling as far as I can tell and for large source values, no corresponding large `byte[]` are created any longer outside of the few remaining spots where we copy to pooled buffers.
closes#102657closes#102030
`StreamInput#readMap()` is quite different from the other `readMap`
overloads, and pairs up with `StreamOutput#writeGenericMap`. This commit
renames it to avoid accidental misuse and so that the names line up
better between writer and reader.
Making this class ref-counted is rather complicated due to its
mutability. As a first step towards a clean solution, move to a single
primary constructor and make more fields `final`. Also this slightly
speeds up and saves memory on deserialisation by not having to create
the fields maps twice.
part of #102030
This are to be made ref-counted shortly. There's no point in having any
pooling/leak-tracking for empty instances though. To prepare for that,
lets add some short-cuts for dealing with empty instances to make the
overall change smaller and cleanup code already.
Same as #101175, shorten `client().prepareIndex(index)` and
`client().prepareIndex().setIndex(index)` via a test utility.
Saves lots of code now and sets up some follow-up simplifcations.
* Avoid "this-escape" by making classes final
The "this-escape" compiler warning is intended to alert
developers to potential bugs in object initialization due to
subclassing. This class of bugs cannot occur when a class is
final. Here, we take cases where a class has no implementations
but generates a "this-escape" warning, and we make those
classes final rather than suppressing the compiler warning.
This makes the remaining suppressions more meaningful, since
they now indicate places where we may want to look for
initialization bugs.
In a few cases, making a class final meant changing some of its
protected fields and methods to private or default
accessibility.
Some classes with no implementations are mocked in testing.
Since making those classes final would involve non-trivial
rewrites of tests, I've left them alone.
* Spotless, remove redundant modifiers, clean up "protected" usage
* Revert a few more mocked classes
Today subclasses of `HandledTransportAction` can specify the executor on
which they run, but the executor is optional and if omitted will use
`DIRECT_EXECUTOR_SERVICE`, which means the action runs on a transport
thread. This is a dangerous default behaviour because it makes it easy
to add new transport actions which implicitly run on a network thread,
which is very hard to pick up in reviews.
This commit makes the executor explicit in all callers, and marks the
dangerous methods for removal.
Adds @SuppressWarnings("this-escape") to all necessary places to that
Elasticsearch can compile with -Werror on JDK21
No investigation has been done to determine whether any of the cases
are a potential source of errors - we have simply suppressed all
existing occurrences.
Resolves: #99845
Remove temporary TransportService#registerRequestHandler functions
that take String instead of Executor and update the callers. This
is part of a larger change to pass Executor into request handlers,
rather than creating and stashing Strings that are only used to
eventually fetch appropriate Executors anyway.
Extensive testing changes were required to make TransportService and
ThreadPool mocks capable of looking up Executors from Strings for
TransportAction constructors. Added a new MockUtils class to
modularize the additional test setup.
Part of the changes for #97879
Remove temporary TransportService#registerRequestHandler functions
that take String instead of Executor and update the callers. This
is part of a larger change to pass Executor into request handlers,
rather than creating and stashing Strings that are only used to
eventually fetch appropriate Executors anyway.
Extensive testing changes were required to make TransportService and
ThreadPool mocks capable of looking up Executors from Strings for
TransportAction constructors. Added a new MockUtils class to
modularize the additional test setup.
Part of the changes for #97879
Constants for TransportVersion currently live alongeside the class
definition. This has been fine since there was only one set of
constants. However, to support serverless, some constants will need to
be defined elsewhere.
This commit moves the existing constants to a new holder class,
TransportVersions. It is almost entirely mechanical, using IntelliJ move
members. The only non mechanical part was slightly shifting how CURRENT
is found, defining a LATEST in TransportVersions that is automatically
calculated (since we already have it, no need to manually define it).
The `StreamOutput` and `StreamInput` APIs are designed so that code
which serializes objects to the transport protocol aligns closely with
the corresponding deserialization code. However today
`StreamOutput#writeCollection` pairs up with a variety of methods on
`StreamInput`, including `readList`, `readSet`, and so on. These methods
are not obviously compatible with `writeCollection` unless you look at
the implementation, and that makes verifying transport protocol code
harder than it needs to be.
This commit renames these methods to `readCollectionAsList`,
`readCollectionAsSet`, and so on, to clarify that they are compatible
with `writeCollection`.
Relates
https://github.com/elastic/elasticsearch/pull/98971#issuecomment-1697289815
We can dry up and shorten a bunch of spots by using the overloads
we already have in the code. Also, added a new overload for writing a
map with string keys.
Noticed this when benchmarking FieldCaps transport messages.
The `writeList` alias just adds more lines to the code and makes
profiling more annoying to read, lets remove it.
Lots of spots where we did weird things around streams like redundant stream creation, redundant collecting
before adding all the collected elements to another collection or so, redundant streams for joining strings
and using less efficient `Collectors.toList` and in a few cases also incorrectly relying on the result being mutable.
Replacing the remaining usages that I could automatically replace
and a couple that I did by hand in this PR.
Also, added the same shortcut to the single node tests to save some
duplication there.
Fixes#82794. Upgrade the spotless plugin, which addresses the issue
around formatting `instanceof` expressions. Formatting of statements
including lambdas seems to have improved too.
The `ActionListener` interface implements its static methods using
various inner classes. These inner classes are public because interfaces
do not support package-private inner classes, although they are `final`
and only have `protected` constructors which prevent anyone else from
instantiating them. It's certainly a mistake for code in other packages
to access these things.
This commit moves the implementation details into a separate class to
hide them from the rest of the world. It also tightens up the calls to
`onFailure()` in a few places to better handle the possibility of this
method throwing an exception. Finally, it moves
`DelegatingActionListener` to a top level public class since this
utility is used in quite a number of places.
Use local-independent `Strings.format` method instead of `String.format(Locale.ROOT, ...)`.
Inline `ESTestCase.forbidden` calls with `Strings.format` for the consistency sake.
Add `Strings.format` alias in `common.Strings`
This commit adds a new test framework for configuring and orchestrating
test clusters for both Java and YAML REST testing. This will eventually
replace the existing "test-clusters" Gradle plugin and the build-time
cluster orchestration.
This exception is only used as random exception in tests and isn't used in
production code since 7.11, so we should remove it. ParsingException or
XContentParseException are preferred choices when parsing location for errors is
available.
This is the continuation of #90176 which leverages #90425 to count query types. This PR adds search usage stats to the existing telemetry by counting sections being used as part of a search request, as well as query types. Each distinct query type is counted once per search request.
The counting is performed while parsing, for the following REST search endpoints:
- _search
- _msearch
- _async_search
- _search/template
- _msearch/template
- _fleet/_fleet_search
- _fleet/_fleet_msearch
All other API using search internally, like reindex, ML transform, rank eval, sql etc. are not counted as part of these search usage stats. Such additional functionalities should have its own dedicated telemetry if needed.
The counting of the search sections is not extensive, only the ones that are interesting to collect counts for are tracked.
The following is the new section added to the cluster stats API response, including some sample stats:
```
"search" : {
"total" : 63,
"sections" : {
"knn" : 42,
"query" : 21,
"aggs" : 46
},
"query" : {
"match" : 58
}
}
```
A big part of the change is actually the plumbing to make a common service class that holds the counters available to all the different callers of the parsing methods, especially plugins. Ideally, there would be a separate component that exposes the search parsing functionality rather than static methods, but changing that would require making the additional component available to the REST layer which is not trivial. I reused the existing UsageService which the RestController already holds, and is already used to count access to the different REST endpoints.
Co-authored-by: Mayya Sharipova mayya.sharipova@elastic.co
Loading of stored fields is currently handled directly in FetchPhase, with
some fairly complex logic examining various bits of the FetchContext to work
out what fields need to be loaded. This is further complicated by synthetic
source, which may have its own stored field requirements.
This commit tries to separate out these concerns a little by adding a new
StoredFieldsSpec record that holds information about which stored fields
need to be loaded. Each FetchSubPhaseProcessor can now report a
StoredFieldsSpec detailing what its requirements are, and these specs can
be merged together, along with requirements from a SourceLoader, to
determine up-front what fields should be loaded by the StoredFieldLoader.
The stored fields themselves are added into the SearchHit by a new
StoredFieldsPhase, which handles alias resolution and value post-
processing. The logic to determine when source should be loaded and
when not, based on the presence of script fields or stored fields, is
moved into FetchContext, which highlights some inconsistencies that
can be fixed in follow-up commits.
This PR affects requests that contain a single index name
or a single pattern (wildcard/datemath).
It aims to systematize the handling of the `allow_no_indices`
and `ignore_unavailable`indices options:
* the allow_no_indices option is to be concerned with
wildcards that expand to nothing (or the entire request
expands to nothing)
* the ignore_unavailable option is to be concerned with
explicit names only (not wildcards)
In addition, the behavior of the above options will now be
independent of the number of expressions in a request.
