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

Emulate "fields" option on older versions #77749

Merged
merged 42 commits into from
Sep 21, 2021

Conversation

cbuescher
Copy link
Member

We introduced the "fields" option in search with version 7.10. With this
change we are trying to do a best-effort attempt at emulating this option
in mixed cluster or CCS scenarios where older nodes or clusters are targeted.

This change emulates the fields behaviour by modifying the search request
to include the respective parts of the "_source" field and then parses them back
into the "fields" section returned with the response. This will not be fully equivalent
to the post-7.10 "fields" functionality (e.g. we cannot get multi-field values which are
not part of “_source” from pre-7.10 nodes), but will include values present in “_source”

In order to use the emulation, clients need to set the enable_fields_emulation flag on
the search request. Also the emulation behaviour will be limited to searches using the
default query_then_fetch search type. If both the “fields” option and source filtering
are mixed in the same search request, we will throw an error.

This PR is a continuation of #75745 with changes to the feature flag serialization.

Christoph Büscher and others added 30 commits July 28, 2021 18:35
We introduced the new "fields" option in search with version 7.10. With this
change we are trying to do a best-effort attempt at emulating this new behaviour
in mixed cluster or CCS scenarios where search requests using the "fields"
option also target older nodes or clusters. In that case, currently we don't
return anything in the "fields" section of the response. This change tried to
emulate the fields behaviour by modifying the request to include the respective
"_source" fields and then parsing them back into the "fields" section on return.
This will not be fully equivalent to the post-7.10 "fields" functionality but at
least try to include whatever we find in "_source" in earlier versions.

Currently Draft only, needs more testing in CCS scenarios but I'm opening this
here already to get some test coverage on the modifications so far.
@cbuescher cbuescher added :Search/Search Search-related issues that do not fall into other categories v7.16.0 labels Sep 15, 2021
@cbuescher cbuescher requested a review from jimczi September 15, 2021 09:21
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Sep 15, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

* use the parameter on the rest request. The bwc emulation will not be necessary on 8.0 any more,
* so this setter will not be availabe there.
*/
@Deprecated
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this to warn that this option will be gone in 8.0 already. Unfortunately we need a public setter in the RestSearchAction when parsing the parameter from the request because the classes are in different packages.

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

LGTM

Also the emulation behaviour will be limited to searches using the
default query_then_fetch search type

Can we add a validation and throw an error if dfs is used ?

@cbuescher
Copy link
Member Author

@elasticmachine run elasticsearch-ci/part-2
@elasticmachine run elasticsearch-ci/packaging-tests-unix-sample

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v7.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants