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

Fix ShardSearchRequest cache key #54071

Merged
merged 2 commits into from
Mar 24, 2020
Merged

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Mar 24, 2020

This commit ensures that we don't use the non-deterministic canReturnNullResponseIfMatchNoDocs boolean in the cache key of the ShardSearchRequest.

The value of this boolean has no influence on the cacheability of the request.

Closes #32827

This commit ensures that we don't use the non-deterministic canReturnNullResponseIfMatchNoDocs
boolean in the cache key of the ShardSearchRequest. The value of this boolean has no influence
on the cacheability of the request.

Closes elastic#32827
@jimczi jimczi added >test Issues or PRs that are addressing/adding tests :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.7.0 labels Mar 24, 2020
@elasticmachine
Copy link
Collaborator

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

Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@mayya-sharipova mayya-sharipova left a comment

Choose a reason for hiding this comment

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

Thanks @jimczi , makes sense

@jimczi jimczi merged commit 54b6872 into elastic:master Mar 24, 2020
@jimczi jimczi deleted the fix_indices_cache_it branch March 24, 2020 17:01
jimczi added a commit that referenced this pull request Mar 24, 2020
This commit ensures that we don't use the non-deterministic canReturnNullResponseIfMatchNoDocs
boolean in the cache key of the ShardSearchRequest. The value of this boolean has no influence
on the cacheability of the request.

Closes #32827
javanna added a commit to javanna/elasticsearch that referenced this pull request Sep 17, 2020
In the context of of a recurring test failure tracked by elastic#32827, we added trace logging and an extra cache key renderer argument to IndicesRequestCache#getOrCompute (see elastic#39475 and elastic#34180).

We addressed the issue with elastic#54071, but the extra argument was left behind, with a NORELEASE comment saying it should be removed.

With this commit, we remove the extra cache key rendered argument and the corresponding log lines which are not so useful without it.

Closes elastic#55837
javanna added a commit that referenced this pull request Sep 18, 2020
In the context of of a recurring test failure tracked by #32827, we added trace logging and an extra cache key renderer argument to IndicesRequestCache#getOrCompute (see #39475 and #34180).

We addressed the issue with #54071, but the extra argument was left behind, with a NORELEASE comment saying it should be removed.

With this commit, we remove the extra cache key rendered argument and the corresponding log lines which are not so useful without it.

Closes #55837
javanna added a commit to javanna/elasticsearch that referenced this pull request Sep 18, 2020
…62534)

In the context of of a recurring test failure tracked by elastic#32827, we added trace logging and an extra cache key renderer argument to IndicesRequestCache#getOrCompute (see elastic#39475 and elastic#34180).

We addressed the issue with elastic#54071, but the extra argument was left behind, with a NORELEASE comment saying it should be removed.

With this commit, we remove the extra cache key rendered argument and the corresponding log lines which are not so useful without it.

Closes elastic#55837
javanna added a commit that referenced this pull request Sep 18, 2020
In the context of of a recurring test failure tracked by #32827, we added trace logging and an extra cache key renderer argument to IndicesRequestCache#getOrCompute (see #39475 and #34180).

We addressed the issue with #54071, but the extra argument was left behind, with a NORELEASE comment saying it should be removed.

With this commit, we remove the extra cache key rendered argument and the corresponding log lines which are not so useful without it.

Closes #55837
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Search/Search Search-related issues that do not fall into other categories >test Issues or PRs that are addressing/adding tests v7.7.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IndicesRequestCacheIT#testQueryRewrite fails
5 participants