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

Handle partial search result with point in time #81349

Merged
merged 7 commits into from
Dec 8, 2021

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Dec 4, 2021

Today, a search request with PIT would fail immediately if any associated indices or nodes are gone, which is inconsistent when allow_partial_search_results is true.

Relates #81256

Today, a search request with PIT would fail immediately if any associated indices are gone, which is inconsistent when allow_partial_search_results is true.
@dnhatn dnhatn added >bug :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.16.1 v8.1.0 labels Dec 4, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Dec 4, 2021
@elasticmachine
Copy link
Collaborator

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

@ywelsch ywelsch self-requested a review December 7, 2021 15:23
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

This PR changes two things:

  • Make PIT work with allow_partial_results = true (previously it would fail)
  • Change http response code when allow_partial_results = false

While the former looks ok for me, I'm worried a bit about the BWC implications of the latter, especially as this is being backported to 7.16.

Should we only do 1) here for the backport and follow-up with 2) in a separate PR?

@dnhatn
Copy link
Member Author

dnhatn commented Dec 7, 2021

Change http response code when allow_partial_results = false

I pushed 8b1be39 to revert the current behaviour when allow_partial_results is false. I am not sure the consequence of this as allow_partial_results defaults to true.

@dnhatn dnhatn requested a review from ywelsch December 7, 2021 19:40
@@ -54,7 +54,7 @@ POST /_search <1>
}
}
--------------------------------------------------
// TEST[catch:missing]
// TEST[catch:unavailable]
Copy link
Member Author

Choose a reason for hiding this comment

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

We have to change this as the allow_partial_results defaults to true.

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM

@dnhatn
Copy link
Member Author

dnhatn commented Dec 8, 2021

Thanks Yannick!

@dnhatn dnhatn merged commit d0d91c6 into elastic:master Dec 8, 2021
@dnhatn dnhatn deleted the point-in-time branch December 8, 2021 15:04
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Dec 8, 2021
Today, a search request with PIT would fail immediately if any 
associated indices or nodes are gone, which is inconsistent when
allow_partial_search_results is true.

Relates elastic#81256
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.0
7.16

dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Dec 8, 2021
Today, a search request with PIT would fail immediately if any 
associated indices or nodes are gone, which is inconsistent when
allow_partial_search_results is true.

Relates elastic#81256
elasticsearchmachine pushed a commit that referenced this pull request Dec 8, 2021
Today, a search request with PIT would fail immediately if any 
associated indices or nodes are gone, which is inconsistent when
allow_partial_search_results is true.

Relates #81256
elasticsearchmachine pushed a commit that referenced this pull request Dec 22, 2021
* Handle partial search result with point in time (#81349)

Today, a search request with PIT would fail immediately if any 
associated indices or nodes are gone, which is inconsistent when
allow_partial_search_results is true.

Relates #81256

* fix tests

* Remove duplicated point-in-time doc

* fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v7.16.1 v8.0.0-rc1 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants