This change fixes a bug introduced in #61779 that uses a compound order to
compare buckets when merging. The bug is triggered when the compound order
uses a primary sort ordered by key (asc or desc).
This commit ensures that we always extract the primary sort when comparing keys
during merging.
The PR is marked as no-issue since the bug has not been released in any official version.
This commit internalizes whether or not a role represents the ability to
contain data. In the future, this will let us remove the compatibility
role notion.
Now that we're consistently using `cat_match` to filter which shards we
run on we can get this confusing case:
1. You have a search with, say, a range and a sub-agg.
2. That search has a query that `can_match` can recognize will match no
docs. On *any* shard.
3. So we dutifully run it on a single shard so it can produce the
"empty" aggs.
4. The shard we pick happens to not have the target of the range mapped.
5. This kicks in the special range aggregator that doesn't collect any
documents.
6. Before this commit, that range aggregator *also* never produced any
sub-aggs.
So, without this change, it was quite possible for a search that
happened to match no documents to "throw away" the sub-aggs of a range
and a few other aggs.
We've had this problem for a long, long time but it is more confusing
now because `can_match` is really kicking in and causing us to see cases
where it looks like you are targeting a lot of shards but you really are
only targeting a couple. It used to be that to get the "no sub-aggs"
behavior you had to explicitly target only shards that didn't map the
target field of the `range` agg. And, like, in that case it isn't too
bad because you targeted a sort of degenerate shard. But now that
`can_match` is doing its thing you can end up with the confusing steps
above. It took me several hours to track down what what happening I know
how the individual pieces of all of this works. It took four hours to
figure out how they fit together in this case....
Anyway! This replaces all the aggregator implementations that throw out
the sub-aggregators with ones that keep them. I think this'll be less
confusing in the future.
Closes#64142
This commit adds logging to indicate whether or not we are using the
bundled JDK. We distinguish between using a distribution that bundles
the JDK versus using a distribution that does not bundle the JDK.
In 7.x we can't just by default generate this setting as it might not be
supported by data nodes that are assigned shards for an older version in mixed version
clusters.
Closes#64152
This commit fixes an issue with the detection on macOS for whether or
not the bundled JDK is being used. The logic between macOS and non-macOS
is different because the JDK has a different directory structure on
macOS versus non-macOS. However, due to notarization issues, we changed
the top-level directory from jdk to jdk.app, yet never updated this
detection logic to account for that.
Ideally, we would have a packaging test that asserts that we have the
behavior here correct, and it maintains over time. Alas, we do not
currently have packaging tests on macOS.
With this change, we will always return the same point in time in a
search response as its input until we implement the retry mechanism
for the point in times.
The formatting of the global bottom value does not take the resolution of the provided
numeric_type into account. This change fixes this bug by providing the resolution
directly in the doc value format if the numeric_type is provided as `date_nanos`.
Closes#63719
We must not remove the snapshot from the initializing set
in the `timeout` getter. This was a plain oversight/mistake
and went unnoticed. It can lead to the removal of a valid
snapshot clone from the cluster state in rare circumstances
(e.g. when a node concurrently joins the cluster or a routing
change happens as it did in the linked test failure).
Closes#64115
If we run into a background merge between creating the snapshot and closing the index
then with compound files we could be in a situation where we get zero file reuse
on restore.
Force merging before the snapshot gives us a single segment that won't change down the line
so the restore always sees file reuse from the close index.
Closes#63476
Assuming the clone failed when the request failed is not sufficient.
There are failure modes where the request fails but the clone still works out
because the data node resent the requeest after the first clone had already been
failed and removed from the cluster state when master was restarted.
Closes#63473
We have to wait for no more operations here not for `1`. This mostly worked
because the test thread would add the listener quickly enough so that it sees the
state where either the snapshot or clone but not both have already finished
but randomly the test thread would be slow and time out on a state without snaphots in it.
testHealthOnMasterFailover could timeout on some of the health requests
in the case where an index is added, since the recovery leads to
extended test run time.
Closes#62690
We had and an error when serializing fully reduced scripted metrics.
Small typo and sever lack of tests..... Anyway, this fixed the one
character typo and adds a bunch more tests.
This commit adds a test in DiskThresholdDeciderTests that verifies
the allocation of a snapshot recovery source based shard in the
situation where the snapshot shard size was successfully provided
by the SnapshotInfoService introduced in #61906 and when the
service failed to provide the size.
Relates #61906
In #57892 I broke *some* sub-aggregations inside of the `parent` and
`child` aggregator, specifically any sub-aggregations that do work in
the `postCollect` phase. This fixes it by delaying the post collect
phase of aggs under `parent` and `child` until `beforeBuildingBuckets`
because, well, we haven't done *any* collection until after that phase.
Currently if distance_feature query contains boost,
it incorrectly gets applied twice: in AbstractQueryBuilder::toQuery and
we also pass this boost to Lucene's LongPoint.newDistanceFeatureQuery.
As a result we get incorrect scores.
This fixes this error to ensure that boost is applied only once.
Closes#63691
This commit fixes the UpdateThreadPoolSettingsTests to be aware of the
hard limit on the maximum size of the system_write executor. This
executor has a hard limit that matches the write executor, which is
the number of allocated processors.
Closes#63131
Backport #63700
Today indexing to a shard with 2147483519 documents will fail that
shard. We should check the number of documents and reject the write
requests instead.
Closes#51136
This fixes a gap in testing and a bug that can occur in various forms:
When we would start a snapshot or clone related to a shard that was done
snapshotting/cloning but its overall operation was not yet finalized
at the time of starting the operation, we would base the operation off of
the wrong generation. This would not cause a corrupted repo, but would
cause the operation to be `PARTIAL`.
This commit fixes the state machine to take into account the correct generation
in this case.
Closes#63498
This PR implements value fetching for the following field types:
* `text` phrase and prefix subfields
* `search_as_you_type`, plus its subfields
* `token_count`, which is implemented by fetching doc values
Supporting these types helps ensure that retrieving all fields through
`"fields": ["*"]` doesn't fail because of unsupported value fetchers.
Currently we flush the Translog buffer when a new operation causes the
buffer to breach 1MB. This introduces a scenario where an exception is
thrown AFTER the writer has accepted the operation. To avoid this, this
commit flushes the Translog in an #add call before adding a new
operation.
This fixes#63299.
This PR adds factory methods for the most common implementations:
* `SourceValueFetcher.identity` to pass through the source value untouched.
* `SourceValueFetcher.toString` to simply convert the source value to a string.
#63214 made TypeFieldType a constant field, and fixed things so that it always
emits deprecation warnings whenever it is referenced in a query or aggregation.
However, it also emits warnings when it is used to build a type filter through
the search context; this is unnecessary, as warnings are already emitted by
the REST layer when types are specified as part of the URL, and it is causing
failures in some BWC tests.
This commit adds a specialised typeFilter method to TypeFieldType to handle
this case without emitted any extra warnings. It also removes an unused duplicate
TypeFieldType class that resulted from a backport merge error.
Fixes#63366
As a result of this, we can remove a chunk of code from TypeParsers as well. Tests
for search/index mode analyzers have moved into their own file. This commit also
rationalises the serialization checks for parameters into a single SerializerCheck
interface that takes the values includeDefaults, isConfigured and the value
itself.
Relates to #62988
We were not consistent in checking for node roles before adding listeners.
In some cases we did check the necessity of a CS listener and in others we did not.
This commit fixes a number of cases of redundant listeners that don't apply to all node roles.
In #61906 we agreed on always providing the default value
ShardRouting.UNAVAILABLE_EXPECTED_SHARD_SIZE
when the SnasphotInfoService failed to retrieve the exact
size for a given snapshot shard. The motivation was to
allow the shard allocation to move forward in case of
failures (so that the unassigned shard does not get stuck
in an unassigned state for too long) while relying on the
fallback values for shard sizes.
Sadly a bug in the
SnapshotShardSizeInfo#getShardSize(ShardRouting, long)
makes the default value to be ignored when the snapshot
shard size retrieval previously failed, returning
ShardRouting.UNAVAILABLE_EXPECTED_SHARD_SIZE
instead of the provided default value. With DiskThresholdDecider
also not relying on the provided default value this triggers
some assertion like in #63376 which helped us to spot the bug.
Closes ##63376
The first refreshDiskUsage() refreshes the ClusterInfo update which in turn
calls listeners like DiskThreshMonitor. This one triggers a reroute as
expected and turns an internal checkInProgress flag before submitting
a cluster state update to relocate shards (the internal flag is toggled
again once the cluster state update is processed).
In the test I suspect that the second refreshDiskUsage() may complete
before DiskThreshMonitor's internal flag is set back to its initial state,
resulting in the second ClusterInfo update to be ignored and message
like "[node_t0] skipping monitor as a check is already in progress" to
be logged. Adding another wait for languid events to be processed
before executing the second refreshDiskUsage() should help here.
Closes#62326
Currently we add translog operation bytes to an array list and flush
them on the next write. Unfortunately, this does not currently play well
with our byte pooling which means each operation is backed, at minimum,
by a 16KB array. This commit improves memory efficiency for small
operations by serializing the operations to an output stream.
Currently a TranslogWriter add operation is synchronized. This operation
adds the bytes to the file output stream buffer and issues a write
system call if the buffer is filled. This happens every 8KB which means
that we routinely block other add calls on system writes.
This commit modifies the add operation to simply place the operation in
an array list. The array list if flushed when the sync call occurs or
when 1MB is buffered.
Plugins are loaded in isolated child class loaders of the root class loader. However, some libraries depend on the context class loader being set. This commit sets the context class loader for the duration of calling each plugins constructor.
relates #52320
Co-authored-by: Ryan Ernst <ryan@iernst.net>
When constructing a value fetcher, the 'parsesArrayValue' flag must match
`FieldMapper#parsesArrayValue`. However there is nothing in code or tests to
help enforce this.
This PR reworks the value fetcher constructors so that `parsesArrayValue` is
'false' by default. Just as for `FieldMapper#parsesArrayValue`, field types must
explicitly set it to true and ensure the behavior is covered by tests.
Follow-up to #62974.
This PR adds deprecation warnings when accessing System Indices via the REST layer. At this time, these warnings are only enabled for Snapshot builds by default, to allow projects external to Elasticsearch additional time to adjust their access patterns.
Deprecation warnings will be triggered by all REST requests which access registered System Indices, except for purpose-specific APIs which access System Indices as an implementation detail a few specific APIs which will continue to allow access to system indices by default:
- `GET _cluster/health`
- `GET {index}/_recovery`
- `GET _cluster/allocation/explain`
- `GET _cluster/state`
- `POST _cluster/reroute`
- `GET {index}/_stats`
- `GET {index}/_segments`
- `GET {index}/_shard_stores`
- `GET _cat/[indices,aliases,health,recovery,shards,segments]`
Deprecation warnings for accessing system indices take the form:
```
this request accesses system indices: [.some_system_index], but in a future major version, direct access to system indices will be prevented by default
```
Determines the shard size of shards before allocating shards that are
recovering from snapshots. It ensures during shard allocation that the
target node that is selected as recovery target will have enough free
disk space for the recovery event. This applies to regular restores,
CCR bootstrap from remote, as well as mounting searchable snapshots.
The InternalSnapshotInfoService is responsible for fetching snapshot
shard sizes from repositories. It provides a getShardSize() method
to other components of the system that can be used to retrieve the
latest known shard size. If the latest snapshot shard size retrieval
failed, the getShardSize() returns
ShardRouting.UNAVAILABLE_EXPECTED_SHARD_SIZE. While
we'd like a better way to handle such failures, returning this value
allows to keep the existing behavior for now.
Note that this PR does not address an issues (we already have today)
where a replica is being allocated without knowing how much disk
space is being used by the primary.
Co-authored-by: Yannick Welsch <yannick@welsch.lu>
Even if we increase the limit it might not take effect straight away if a thread is
blocked on a long wait in `org.elasticsearch.index.snapshots.blobstore.RateLimitingInputStream#maybePause`.
Let's increase the limit a little and see if that deals with the remaining failures for good and stop burning
cycles busy asserting a future completion.
Closes#63246
MapperService carries a lot of weight and is only used to determine if loading of field data for the id field is enabled, which can be done in a different way.
Just a few spots where we can dry up these tests using the snapshot test infrastructure
in core that I found while studying the existing searchable snapshot tests.
In #62509 we already plugged faster sequential access for stored fields in the fetch phase.
This PR now adds using the potentially better field reader also in SourceLookup.
Rally exeriments are showing that this speeds up e.g. when runtime fields that are using
"_source" are added e.g. via "docvalue_fields" or are used in queries or aggs.
Closes#62621
In 6x and 7x, indexes can have only one type, which means that we can rework
all queries against the type field to use a ConstantFieldType. This has already
been done in master with the removal of the TypeFieldMapper, but we still need
that class in 7x to deal with nested documents. This commit leaves
TypeFieldMapper in place, but refactors TypeFieldType to extend
ConstantFieldType and consolidates deprecation warnings within that class.
It also incidentally removes the requirement to pass a MapperService to
IndexFieldData.Builder#build, which should allow #63197 to be backported.
There is no need to let snapshots that haven't yet written anything to the repo
finalize with `FAILED`. When we still had the `INIT` state we would also just remove
these snapshots from the state without any further action.
This is not just a theoretical optimization. Currently, the situation of having a lot of
queued up snapshots is fairly complicated to resolve when all the queued shards move to aborted
since it is now necessary to execute tasks on the `SNAPSHOT` pool (that might be very busy) to
remove the snapshot from the CS (including a number of redundant CS updates and repo writes
for finalizing these snapshots before deleting them right away after).
If the connection between clusters is disconnected or the leader cluster
is offline, then CCR shard-follow tasks can stop with "no seed node
left". CCR should retry on this error.
The copy constructors previously used were hard to read and the exact state changes
were not obvious at all.
Refactored those into a number of named constructors instead, added additional assertions
and moved the snapshot abort logic into `SnapshotsInProgress`.
In #63242 we changed how we build `nextRoundingValue` to, well, be
correct. But the old `org.elasticsearch.common.rounding.Rounding`
implementation didn't get the fix. Which is fine, because it doesn't
that method on that implementation doesn't receive any use outside of
tests. In fact, it is entirely removed in master. Anyway, now that the
two implementation produce different values we really can't go around
asserting that they produce the same values now can we? Well, we were!
This skips that assertion if we know `nextRoundingValue` is implemented
differently.
Closes#63256
* Setting `script.painless.regex.enabled` has a new option,
`use-factor`, the default. This defaults to using regular
expressions but limiting the complexity of the regular
expressions.
In addition to `use-factor`, the setting can be `true`, as
before, which enables regular expressions without limiting them.
`false` totally disables regular expressions, which was the
old default.
* New setting `script.painless.regex.limit-factor`. This limits
regular expression complexity by limiting the number characters
a regular expression can consider based on input length.
The default is `6`, so a regular expression can consider
`6` * input length number of characters. With input
`foobarbaz` (length `9`), for example, the regular expression
can consider `54` (`6 * 9`) characters.
This reduces the impact of exponential backtracking in Java's
regular expression engine.
* add `@inject_constant` annotation to whitelist.
This annotation signals that a compiler settings will
be injected at the beginning of a whitelisted method.
The format is `argnum=settingname`:
`1=foo_setting 2=bar_setting`.
Argument numbers must start at one and must be sequential.
* Augment
`Pattern.split(CharSequence)`
`Pattern.split(CharSequence, int)`,
`Pattern.splitAsStream(CharSequence)`
`Pattern.matcher(CharSequence)`
to take the value of `script.painless.regex.limit-factor` as a
an injected parameter, limiting as explained above when this
setting is in use.
Fixes: #49873
Backport of: 93f29a4
We only ever use this with `XContentParser` no need to make it inline
worse by forcing the lambda and hence dynamic callsite here.
=> Extraced the exception formatting code path that is likely very cold
to a separate method and removed the lambda usage in hot loops by simplifying
the signature here.
Small refactoring to shorten the diff with the clone logic in #61839:
* Since clones will create a different kind of shard state update that
isn't the same request sent by the snapshot shards service (and cannot be
the same request because we have no `ShardId`) base the shard state updates
on a different class that can be extended to be general enough to accomodate
shard clones as well.
* Make the update executor a singleton (can't make it an inline lambda as that
would break CS update batching because the executor is used as a map key but
this change still makes it crystal clear that there's no internal state to the
executor)
* Make shard state update responses a singleton (can't use TransportResponse.Empty because
we need an action response but still it makes it clear that there's no actual
response with content here)
* Just some obvious drying up of these super complex tests.
* Mainly just shortening the diff of #61839 here by moving test utilities
to the abstract test case.
Also, making use of the now available functionality to simplify existing tests
and improve logging in them.
"interval" style roundings were implementing `nextRoundingValue` in a
fairly inconsistent way - it'd produce a value, but sometimes that
value would be the same as the previous rounding value. This makes it
consistently the next value that `rounding` would make.
As long as `bestEffortConsistency` is `true`, the value of `latestKnownRepoGen`
can be updated as a result of reads. We can only assert that `latestKnownRepoGen`
and cluster state move in lock-step if `bestEffortConsistency` was `false` before
updating the metadata generation as well as after.
Closes#62877
There is a small race when processing the cluster state that is used to
establish a newly elected leader as master of the cluster: it can pick the term
in its master state update task from a different (newer) election. This trips
an assertion in `Coordinator.publish(...)` where we claim that the term on the
state allows to uniquely define the pre-state but this isn't so. There are no
bad consequences of this race since such a publication fails later on anyway.
This PR fixes things so that the assertion holds true by improving the handling
of terms during cluster state processing by associating each master state
update task that is used to establish a newly elected leader with the correct
corresponding term from its election. It also explicitly handles the case where
the pre-state that is used as base state has already superseded the current
state. As a nice side-effect, join batching now only happens based on the same
term.
Closes#61437
The iteration over `timeoutClusterStateListeners` starts when the CS applier
thread is still running. This can lead to entries being added to it that never
get their listener resolved on shutdown and thus leak that listener as observed
in a stuck test in #62863.
Since `listener.onClose()` is idempotent we can just call it if we run into a stopped service
on the CS thread to avoid the race with certainty (because the iteration in `doStop` starts after
the stopped state has been set).
Closes#62863
For runtime fields, we will want to do all search-time interaction with
a field definition via a MappedFieldType, rather than a FieldMapper, to
avoid interfering with the logic of document parsing. Currently, fetching
values for runtime scripts and for building top hits responses need to
call a method on FieldMapper. This commit moves this method to
MappedFieldType, incidentally simplifying the current call sites and freeing
us up to implement runtime fields as pure MappedFieldType objects.
In #22721, the decision to throttle indexing was inadvertently flipped,
so that we until this commit throttle indexing during recovery but
never throttle user initiated indexing requests. This commit
fixes that to throttle user initiated indexing requests and never
throttle recovery requests.
Closes#61959
Backport #63170 to 7.x branch.
The _index field is a special field that allows using queries against the name of an index or alias.
Data stream names were not included, this pr fixes that by changing SearchIndexNameMatcher
(which used via IndexFieldMapper) to also include data streams.
Splitting some tests out of this class that has become a catch-all
for random snapshot related tests into either existing suits that fit
better for these tests or one of two new suits to prevent timeouts
in extreme cases (e.g. `WindowsFS` + many nodes + multiple data paths per node).
No other changes to tests were made whatsoever.
Closes#61541
7.x client can pass media type with a version which will return a 7.x
version of the api in ES 8.
In ES server 7 this media type shoulld be accepted but it serve the same
version of the API (7x)
relates #61427
* Add System Indices check to AutoCreateIndex
By default, Elasticsearch auto-creates indices when a document is
submitted to a non-existent index. There is a setting that allows users
to disable this behavior. However, this setting should not apply to
system indices, so that Elasticsearch modules and plugins are able to
use auto-create behavior whether or not it is exposed to users.
This commit constructs the AutoCreateIndex object with a reference to
the SystemIndices object so that we bypass the check for the user-facing
autocreate setting when it's a system index that is being autocreated.
We also modify the logic in TransportBulkAction to make sure that if a
system index is included in a bulk request, we don't skip the
autocreation step.
Jim Ferenczi
markharwood
Ignacio Vera
James Rodewig
Armin Braun
Jason Tedor
Yang Wang
Nik Everett
Nhat Nguyen
Marios Trivyzas
Przemyslaw Gomulka
Henning Andersen
Tanguy Leroux
Mayya Sharipova
Ioannis Kakavas
Jay Modi
Lee Hinman
Julie Tibshirani
Tim Brooks
Dan Hermann
Christoph Büscher
Alan Woodward
Dawid Weiss
Gordon Brown
Luca Cavanna
Stuart Tettemer
David Roberts
Yannick Welsch
nitin2goyal
Martijn van Groningen
William Brafford