Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Add ResolvedExpression wrapper (#114592)" #115317

Merged
merged 2 commits into from
Oct 22, 2024

Conversation

original-brownbear
Copy link
Member

This reverts commit 4c15cc0.
This commit introduced an orders of magnitude regression when searching many shards.
Small Rally excerpt:

image

@original-brownbear original-brownbear added >bug :Data Management/Indices APIs APIs to create and manage indices and templates labels Oct 22, 2024
@elasticsearchmachine elasticsearchmachine added v9.0.0 Team:Data Management Meta label for data/management team labels Oct 22, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticsearchmachine
Copy link
Collaborator

Hi @original-brownbear, I've created a changelog YAML for you.

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Mary said she would look into this tomorrow also.

@original-brownbear
Copy link
Member Author

Thanks Lee!

@original-brownbear original-brownbear added auto-backport Automatically create backport pull requests when merged auto-merge labels Oct 22, 2024
@original-brownbear original-brownbear merged commit d9baf6f into elastic:main Oct 22, 2024
16 checks passed
@original-brownbear original-brownbear deleted the revert-iner branch October 22, 2024 15:43
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.16

original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Oct 22, 2024
)

This reverts commit 4c15cc0.
This commit introduced an orders of magnitude regression when searching many shards.
@dakrone
Copy link
Member

dakrone commented Oct 22, 2024

@original-brownbear this needs to go to 8.17 (the 8.x branch) also

@original-brownbear
Copy link
Member Author

Ah right thanks!

@dakrone
Copy link
Member

dakrone commented Oct 22, 2024

💚 All backports created successfully

Status Branch Result
8.x

Questions ?

Please refer to the Backport tool documentation

@dakrone
Copy link
Member

dakrone commented Oct 22, 2024

It appears that adding the v8.17.0 label didn't automatically backport this, so I did it manually.

dakrone pushed a commit to dakrone/elasticsearch that referenced this pull request Oct 22, 2024
)

This reverts commit 4c15cc0.
This commit introduced an orders of magnitude regression when searching many shards.

(cherry picked from commit d9baf6f)
elasticsearchmachine pushed a commit that referenced this pull request Oct 23, 2024
This reverts commit 4c15cc0.
This commit introduced an orders of magnitude regression when searching many shards.

(cherry picked from commit d9baf6f)

Co-authored-by: Armin Braun <me@obrown.io>
elasticsearchmachine pushed a commit that referenced this pull request Oct 23, 2024
This reverts commit 4c15cc0.
This commit introduced an orders of magnitude regression when searching many shards.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@original-brownbear
Copy link
Member Author

Thanks again Lee!

georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Oct 25, 2024
)

This reverts commit 4c15cc0.
This commit introduced an orders of magnitude regression when searching many shards.
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Nov 4, 2024
)

This reverts commit 4c15cc0.
This commit introduced an orders of magnitude regression when searching many shards.
jbaiera referenced this pull request in jbaiera/elasticsearch Nov 20, 2024
**Introduction**

> In order to make adoption of failure stores simpler for all users, we
are introducing a new syntactical feature to index expression
resolution: The selector. > > Selectors, denoted with a :: followed by a
recognized suffix will allow users to specify which component of an
index abstraction they would like to operate on within an API call. In
this case, an index abstraction is a concrete index, data stream, or
alias; Any abstraction that can be resolved to a set of indices/shards.
We define a component of an index abstraction to be some searchable unit
of the index abstraction. > > To start, we will support two components:
data and failures. Concrete indices are their own data components, while
the data component for index aliases are all of the indices contained
therein. For data streams, the data component corresponds to their
backing indices. Data stream aliases mirror this, treating all backing
indices of the data streams they correspond to as their data component.
>  > The failure component is only supported by data streams and data
stream aliases. The failure component of these abstractions refer to the
data streams' failure stores. Indices and index aliases do not have a
failure component.

For more details and examples see
elastic#113144. All this work has
been cherry picked from there.

**Purpose of this PR**

This PR is introducing a wrapper around the resolved expression that
used to be a `String` to create the base on which the selectors are
going to be added.

The current PR is just a refactoring and does not and should not change
any existing behaviour.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged auto-merge >bug :Data Management/Indices APIs APIs to create and manage indices and templates Team:Data Management Meta label for data/management team v8.16.0 v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants