This PR addresses issues around aggregations cancellation, mentioned in https://github.com/elastic/elasticsearch/issues/108701 and other places. In brief, during aggregations collection time, we respect cancellation via the mechanisms in the searcher to poison cancelled queries. But once the aggregation finishes collection, there is no further need to interact with the searcher, so we cannot rely on that for cancellation checking. In particular, deeply nested aggregations can spend a long time constructing the results tree.
Checking for cancellation is a trade off, as the check itself is somewhat expensive (it involves a volatile read), so we want to balance checking often enough that cancelled queries aren't taking up resources for a long time, but not so frequently that it slows down most aggregation queries. Our first attempt to this is to check once when we go to build sub-aggregations, as the worst cases for this that we've seen involve needing to build deep sub-aggregation trees. Checking at sub-aggregation construction time also provides a conveniently centralized method call to add the check to.
---------
Conflicts:
server/src/main/java/org/elasticsearch/search/aggregations/bucket/BucketsAggregator.java
test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java
Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
Under very unfortunate conditions tests that check xContent objects
roundtrip parsing (like i.e. SearchHitsTests#testFromXContent)
can fail when we happen to randomly pick YAML xContent type and create
random (realistic)Unicode character sequences that may contain the
character U+0085 (133) from the Latin1 code page. That specific character
doesn't get parsed back to its original form for YAML xContent, which can
lead to rare but hard to diagnose test failures.
This change adds logic to AbstractXContentTestCase#test() which lies at
the core of most of our xContent roundtrip tests that disallows test
instances containing that particular character when using YAML xContent
type.
Closes#97716
* Test ML model server (#120270)
* Fix model downloading for very small models.
* Test MlModelServer
* Tiny ELSER
* unmute TextEmbeddingCrudIT and DefaultEndPointsIT
* update ELSER
* Improve MlModelServer
* tiny E5
* more logging
* improved E5 model
* tiny reranker
* scan for ports
* [CI] Auto commit changes from spotless
* Serve default models when optimized model is requested
* @ClassRule
* polish code
* Respect dynamic setting ML model repo
* fix metadata for optimized models
* improve logging
---------
Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
* backport HttpHeaderParser
* Fix stripping platform
---------
Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
This PR updates `bc-fips` and `bctls-fips` dependencies to the latest
minor versions.
(cherry picked from commit 6ea3e01958)
Co-authored-by: Slobodan Adamović <slobodanadamovic@users.noreply.github.com>
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)
# Conflicts:
# build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/distribution/DockerCloudElasticsearchDistributionType.java
# build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/distribution/DockerUbiElasticsearchDistributionType.java
# build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/Fixture.java
# plugins/repository-hdfs/hadoop-client-api/build.gradle
# server/src/main/java/org/elasticsearch/inference/ChunkingOptions.java
# x-pack/plugin/kql/build.gradle
# x-pack/plugin/migrate/build.gradle
# x-pack/plugin/security/qa/security-basic/build.gradle
[esql] > Unexpected error from Elasticsearch: illegal_state_exception - sink exchanger for id [ruxoDDxXTGW55oIPHoCT-g:964613010] already exists.
This issue occurs when two or more clusterAliases point to the same
physical remote cluster. The exchange service assumes the destination is
unique, which is not true in this topology. This PR addresses the
problem by appending a suffix using a monotonic increasing number,
ensuring that different exchanges are created in such cases.
Another issue arising from this behavior is that data on a remote
cluster is processed multiple times, leading to incorrect results. I can
work on the fix for this once we agree that this is an issue.
We don't seem to have a test that completely verifies that a S3
repository can reload credentials from an updated keystore. This commit
adds such a test.
Backport of #116762 to 8.16.
The fetch phase is subject to timeouts like any other search phase. Timeouts
may happen when low level cancellation is enabled (true by default), hence the
directory reader is wrapped into ExitableDirectoryReader and a timeout is
provided to the search request.
The exception that is used is TimeExceededException, but it is an internal
exception that should never be returned to the user. When that is thrown, we
need to catch it and throw error or mark the response as timed out depending
on whether partial results are allowed or not.
A `CompleteMultipartUpload` action may fail after sending the `200 OK`
response line. In this case the response body describes the error, and
the SDK translates this situation to an exception with status code 0 but
with the `ErrorCode` string set appropriately. This commit enhances the
exception handling in `S3BlobContainer` to handle this possibility.
Closes#102294
Co-authored-by: Pat Patterson <metadaddy@gmail.com>
* Allow for queries on _tier to skip shards during coordinator rewrite (#114990)
The `_tier` metadata field was not used on the coordinator when
rewriting queries in order to exclude shards that don't match. This lead
to queries in the following form to continue to report failures even
though the only unavailable shards were in the tier that was excluded
from search (frozen tier in this example):
```
POST testing/_search
{
"query": {
"bool": {
"must_not": [
{
"term": {
"_tier": "data_frozen"
}
}
]
}
}
}
```
This PR addresses this by having the queries that can execute on `_tier`
(term, match, query string, simple query string, prefix, wildcard)
execute a coordinator rewrite to exclude the indices that don't match
the `_tier` query **before** attempting to reach to the shards (shards,
that might not be available and raise errors).
Fixes#114910
* Don't use getFirst
* Test compile
Long GC disruption relies on Thread.resume, which is removed in JDK 23.
Tests that use it predate more modern disruption tests. This commit
removes gc disruption and the master disruption tests. Note that tests
relying on this scheme have already not been running since JDK 20 first
deprecated Thread.resume.
Here we check for the existence of a `host.name` field in index sort settings
when the index mode is `logsdb` and decide to inject the field in the mapping
depending on whether it exists or not. By default `host.name` is required for
sorting in LogsDB. This reduces the chances for errors at mapping or template
composition time as a result of injecting the `host.name` field only if strictly
required. A user who wants to override index sort settings without including
a `host.name` field would be able to do so without finding an additional
`host.name` field in the mappings (injected automatically). If users override the
sort settings and a `host.name` field is not included we don't need
to inject such field since sorting does not require it anymore.
As a result of this change we have the following:
* the user does not provide any index sorting configuration: we are responsible for injecting the default sort fields and their mapping (for `logsdb`)
* the user explicitly provides non-empty index sorting configuration: the user is also responsible for providing correct mappings and we do not modify index sorting or mappings
Note also that all sort settings `index.sort.*` are `final` which means doing this
check once, when mappings are merged at template composition time, is enough.
(cherry picked from commit 9bf6e3b0ba)
JNA has a static thread which handles cleaning up native memory
references. This commit adds the thread name to those filtered out of
thread leak detection since it lives for the lifetime of the JDK (yet
might be started in the middle of a test).
closes#114555
* Create a fluent builder to help implement ChunkedToXContent (#112389)
Rather than manually adding startObject/endObject, and having to line everything up manually, this handles the start/end for you.
A few implementations are converted already. In the long run, I would like this to replace ChunkedXContentHelper.
* Convert a few more implementations to ChunkedXContentBuilder (#113125)
Remove the complex methods from ChunkedXContentHelper
* Further conversions to ChunkedXContentBuilder (#114237)
---------
Co-authored-by: Simon Cooper <simon.cooper@elastic.co>
The ST_DISTANCE function added in #108764 was optimized for lucene pushdown in a series of followup PRs, but this did not include sorting by distance. Now this is resolved, for two key scenarios, both known to be valued by users:
* Sorting by distance:
`FROM index | EVAL distance=ST_DISTANCE(field, literal) | SORT distance`
* Sorting and filtering by distance:
`FROM index | EVAL distance=ST_DISTANCE(field, literal) | WHERE distance < literal | SORT distance`
The key changes required to make this work:
* Add to the EsQueryExec the appropriate sort->_geo_distance sort type
* Enhance PushTopNToSource to understand how to pushdown the sort even when there is an EVAL in between the FROM and the SORT (between the TopNExec and the EsQueryExec in the physical plan).
* Enhance PushFiltersToSource to understand how to pushdown the filter even when there is an EVAL in between the FROM and the WHERE (between the Filter and the EsQueryExec in the physical plan).
A useful bonus feature of this additional EVAL intelligence is that other, non-spatial cases are now also pushed down. In particular EVALs that are simple aliases are considered and pushed down, for both filtering and sorting.
Local benchmark results, very approximate, but show massive improvements for distanceSort and distanceFilterSort, which relate to the two cases listed above.
Benchmark Query DSL ESQL before this PR ESQL after this PR Comments
distanceFilter 10 5 5 Optimized in #109972
distanceEvalFilter 10 10000 1500 Still slow due to unnecessary EVAL
distanceSort 150 12000 160
distanceFilterSort 20 10000 24
NOTE: This enables pushing down sorting by any ReferenceAttribute that either refers to a sortable FieldAttribute, or to an StDistance function that itself refers to a suitable FieldAttribute of geo_point type.
---------
Co-authored-by: Alexander Spies <alexander.spies@elastic.co>
`ThreadContext#stashContext` doesn't guarantee to give a clean thread
context, but it's important we don't allow the callers' thread contexts
to leak into the cluster state update. This commit captures the desired
thread context at startup rather than using `stashContext` when forking
the processor.
Delay construction of `Warnings` until they are needed to save memory
when evaluating many many many expressions. Most expressions won't use
warnings at all and there isn't any need to make registering warnings
super duper fast. So let's make the construction lazy to save a little
memory. It's like 200 bytes per expression which isn't much, but it's
possible to have thousands of expressions in a single query. Abusive,
but possible.
This also consolidates all `Warnings` usages to a single `Warnings`
class. We had two. We don't need two.
Currently we have a relatively basic decider about when to throttling
indexing. This commit adds two levels of watermarks with configurable
bulk size deciders. Additionally, adds additional settings to control
primary, coordinating, and replica rejection limits.
