After tsid hashing was introduced (#98023), the time series aggregator generates the tsid (from all dimension fields) instead of using the value from the _tsid field directly. This generation of the tsid happens for every time serie, parent bucket and segment combination.
This changes alters that by only generating the tsid once per time serie and segment. This is done by just locally recording the current tsid.
This change ensures that the matches implementation of the `SourceConfirmedTextQuery` only checks the current document instead of calling advance on the two phase iterator. The latter tries to find the first doc that matches the query instead of restricting the search to the current doc. This can lead to abnormally slow highlighting if the query is very restrictive and the highlight is done on a non-matching document.
Closes#103298
Today a node with a registered `URLRepository` will not shut down
cleanly because it never releases the last of the `activityRefs`. This
commit fixes that.
We could still be manipulating a network message when the event loop
shuts down, causing us to close the message while it's still in use.
This is at best going to be a little surprising to the caller, and at
worst could be an outright use-after-free bug.
This commit moves the double-check for a leaked promise to happen
strictly after the event loop has fully terminated, so that we can be
sure we've finished using it by this point.
Relates #105306, #97301
Similar to #99392, #97879 etc, no need to have the
`NodePersistentTasksExecutor` look up the executor to use each time, nor
does it necessarily need to use a named executor from the `ThreadPool`.
This commit pulls the lookup earlier in initialization so we can just
use a bare `Executor` instead.
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 single-char variant of String.indexOf() where possible
indexOf(char) is more efficient than searching for the same one-character String.
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
* 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.
We want to report that observation of document parsing has finished only upon a successful indexing.
To achieve this, we need to perform reporting only in one place (not as previously in both IngestService and 'bulk action')
This commit splits the DocumentParsingObserver in two. One for wrapping an XContentParser and returning the observed state - the DocumentSizeObserver and a DocumentSizeReporter to perform an action when parsing has been completed and indexing successful.
To perform reporting in one place we need to pass the state from IngestService to 'bulk action'. The state is currently represented as long - normalisedBytesParsed.
In TransportShardBulkAction we are getting the normalisedBytesParsed information and in the serverless plugin we will check if the value is indicating that parsing already happened in IngestService (value being != -1) we create a DocumentSizeObserver with the fixed normalisedBytesParsed and won't increment it.
When the indexing is completed and successful we report the observed state for an index with DocumentSizeReporter
small nit: by passing the documentParsingObserve via SourceToParse we no longer have to inject it via complex hierarchy for DocumentParser. Hence some constructor changes
In this PR we are refactoring the internals of the `IndicesOptions` class. Because this class is widely used the refactoring is strictly an internal refactoring, we do not change the existing serialisation. This allows us to better test this and to preserve performance over the wire. The improvements we are brining forth with this PR are:
- New internal structure of the flags, based on what the flags influnce.
- Every flag is a boolean instead of using the presence of an enum options in a set.
- We provide builders to allow easier construction of the object and easier overriding of the defaults.
- This will enable easier extension that might be useful for other projects.
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.
Similar to #97301, the fix in #105293 was still not quite correct: we
could in principle shut down the transport after checking `isOpen()` but
before sending the message. Applying the same fix as for the transport
layer here.
The metrics of HTTP request time can be unavailable (null) when the
request fails on the client side. This PR makes sure we do not attempt
to record it when it happens to avoid NPE.
Today a `HttpResponse` is always released via a `ChannelPromise` which
means the release happens on a network thread. However, it's possible we
try and send a `HttpResponse` after the node has got far enough through
shutdown that it doesn't have any running network threads left, which
means the response just leaks.
This is no big deal in production, it becomes irrelevant when the
process exits, but in tests we start and stop many nodes within the same
process so mustn't leak anything.
At this point in shutdown, all HTTP channels are now closed, so it's
sufficient to check whether the channel is open first, and to fail the
listener on the calling thread if not. That's what this commit does.
Closes#104651
We are adding a query parameter to the field_caps api in order to filter out
fields with no values. The parameter is called `include_empty_fields` and
defaults to true, and if set to false it will filter out from the field_caps
response all the fields that has no value in the index.
We keep track of FieldInfos during refresh in order to know which field has
value in an index. We added also a system property
`es.field_caps_empty_fields_filter` in order to disable this feature if needed.
---------
Co-authored-by: Matthias Wilhelm <ankertal@gmail.com>
To improve cross-cluster search user experience, Kibana needs an endpoint that is accessible
by arbitrary Kibana dashboard search users and provides:
1. a listing of clusters in scope for a CCS query (based on the index expression and whether
there are any indices on each cluster that the Kibana user has access to query).
2. whether that cluster is currently connected to the querying cluster (will it come back as
skipped or failed in a CCS search)
3. showing the skip_unavailable setting for those clusters (so you can know whether it will
return skipped or failed in a CCS search)
4. the ES version of the cluster
Since no single Elasticsearch endpoint provides all of these features, this PR creates a new endpoint `_resolve/cluster` that works along side the existing `_resolve/index` endpoint
(and leverages some of its features).
Example usage against a cluster with 2 remote clusters configured:
GET /_resolve/cluster/*,remote*:bl*
Response:
{
"(local)": {
"connected": true,
"skip_unavailable": false,
"matching_indices": true,
"version": {
"number": "8.12.0-SNAPSHOT",
"build_flavor": "default",
"minimum_wire_compatibility_version": "7.17.0",
"minimum_index_compatibility_version": "7.0.0"
}
},
"remote2": {
"connected": true,
"skip_unavailable": true,
"matching_indices": true,
"version": {
"number": "8.12.0-SNAPSHOT",
"build_flavor": "default",
"minimum_wire_compatibility_version": "7.17.0",
"minimum_index_compatibility_version": "7.0.0"
}
},
"remote1": {
"connected": true,
"skip_unavailable": false,
"matching_indices": false,
"version": {
"number": "8.12.0-SNAPSHOT",
"build_flavor": "default",
"minimum_wire_compatibility_version": "7.17.0",
"minimum_index_compatibility_version": "7.0.0"
}
}
}
Almost all errors show up as "error" entries in the response.
Only the local SecurityException returns a 403 since that happens before the ResolveCluster
Transport code kicks in.
If the client closes the channel while we're in the middle of a chunked
write then today we don't complete the corresponding listener. This
commit fixes the problem.
This reduces the risk of document loss if too many fields are added.
As these component templates are imported by Fleet, this also affects
integrations.
This PR extends the repository integrity health indicator to cover also unknown and invalid repositories. Because these errors are local to a node, we extend the `LocalHealthMonitor` to monitor the repositories and report the changes in their health regarding the unknown or invalid status.
To simplify this extension in the future, we introduce the `HealthTracker` abstract class that can be used to create new local health checks.
Furthermore, we change the severity of the health status when the repository integrity indicator reports unhealthy from `RED` to `YELLOW` because even though this is a serious issue, there is no user impact yet.
The unconditional rollover that is a consequence of a lazy rollover command is triggered by the creation of a document. In many cases, the user triggering this rollover won't have sufficient privileges to ensure the successful execution of this rollover. For this reason, we introduce a dedicated rollover action and a dedicated internal user to cover this case and enable this functionality.
We are building a list of InternalAggregations from a list of Buckets, therefore we can use an AbstractList to create the actual list and save some allocations.
Today, we're counting all mappers, including mappers for subfields that
aren't explicitly added to the mapping towards the field limit.
This means that some field types, such as `search_as_you_type` or
`percolator` count as more than one field even though that's not
apparent to users as they're just defining them as a single field in the
mapping.
This change makes it so that each field mapper only counts as one. We're
still counting multi-fields.
This makes it easier to understand for users why the field limit is hit.
~In addition to that, it also simplifies
https://github.com/elastic/elasticsearch/pull/96235 as it makes the
implementation of `Mapper.Builder#getTotalFieldsCount` much easier and
easier to align with `Mapper#getTotalFieldsCount`. This reduces the risk
of over- or under-estimating the field count of a `Mapper.Builder` in
`DocumentParserContext#addDynamicMapper`, which in turn reduces the risk
of data loss due to the issue described here:
https://github.com/elastic/elasticsearch/pull/96235#discussion_r1402495749.~
*Edit: due to https://github.com/elastic/elasticsearch/pull/103865, we
don't need an implementation of `getTotalFieldsCount` or `mapperSize` in
`Mapper.Builder`. Still, this PR more closely aligns
`Mapper#getTotalFieldsCount` with `MappingLookup#getTotalFieldsCount`,
which `DocumentParserContext#addDynamicMapper` uses to determine
whether the field limit is hit*
A potential risk of this is that we're now effectively allowing more
fields in the mapping. It may be surprising to users that more fields
can be added to a mapping. Although, I'd not expect negative
consequences from that. Generally, I'd expect users to be happy about
any change that reduces the risk of data loss.
We could also think about whether to apply the new counting logic only
to new indices (depending on the `IndexVersion`). However, that would
add more complexity and I'm not convinced about the value. We'd then
need to maintain two different ways of counting fields and also require
passing in the `IndexVersion` to `MappingLookup` which previously didn't
require the `IndexVersion`.
This PR is meant as a conversation starter. It would also simplify
https://github.com/elastic/elasticsearch/pull/96235 but I don't think
this blocks that PR in any way.
I'm curious about the opinion of @javanna and @jpountz on this.
This PR updates the ingest service to detect if a failed ingest document was bound for a data stream configured
with a failure store, and in that event, restores the document to its original state, transforms it with its failure
information, and redirects it to the failure store for the data stream it was originally targeting.
We can avoid building composite byte buf instances on the transport
layer (they have quite a bit of overhead and make heap dumps more
complicated to read). There's no need to add another round of references
to the BytesReference components here. Just write these out as they come
in. This would allow for some efficiency improving follow-ups where we
can essentially release the pages that have passed the write pipeline.
To avoid having this explode the size of the queue for writes per
channel, I moved that to a linked list. The slowdown from a linked list
is irrelevant I believe. Mostly the queue is empty so it doesn't matter
or if it isn't empty, operations other than dequeuing are much more
important to performance in this logic anyway (+ Netty internally uses a
LL down the line anyway).
I would regard this as step-1 in making the serialisation here more lazy
like on the REST layer to avoid copying bytes to the outbound buffer
that we already have as `byte[]`.
