-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
IndicesRequestCacheIT#testQueryRewrite fails #32827
Comments
Pinging @elastic/es-search-aggs |
Although on a different test method in the same test class, I think this is the same failure: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+matrix-java-periodic/ES_BUILD_JAVA=java11,ES_RUNTIME_JAVA=java11,nodes=virtual&&linux/265/console . I can't repro either.
|
* Adds code to help with IndicesRequestCacheIT failures Relates to #32827 * Adds comment * Fixes test failure
I merged #33313 last week that should hopefully fix this and if not sure provide better information if it occurs again. I'm going to close this issue for now but please re-open if the issue re-occurs |
@colings86 after days of silence we have got two new failures in the last 10 hours. Could you take a look if they provide more information: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+release-tests/33/console |
I opened #34180 to add some more logging information and hopefully see whats going on better |
#34180 is now merged. Please ping me if this fails again as the PR only adds debugging information. |
* Adds trace logging to IndicesRequestCache This change adds trace level logging to `IndicesrrequestCache` witht eh primary aim of helping to identify the cause of teh failures in #32827. The cache will log at trace level when a cache hit or miss occurs including the reader version and the cache key. Note that this change adds a `cacheKeyRenderer` whcih supplies a human readable String of the cache key since the actual cache key itself is a `BytesReference` containing the wire protocol serialised form of the request. Logging is also added for the case where a search timeout occurs and fr that reason the cache entry is invalidated. * Adds comment to remaind us to remove cacheKeyRenderer
* Adds trace logging to IndicesRequestCache This change adds trace level logging to `IndicesrrequestCache` witht eh primary aim of helping to identify the cause of teh failures in #32827. The cache will log at trace level when a cache hit or miss occurs including the reader version and the cache key. Note that this change adds a `cacheKeyRenderer` whcih supplies a human readable String of the cache key since the actual cache key itself is a `BytesReference` containing the wire protocol serialised form of the request. Logging is also added for the case where a search timeout occurs and fr that reason the cache entry is invalidated. * Adds comment to remaind us to remove cacheKeyRenderer
Quick new datapoint on this, I just did a search in the recent CI failures and couldn't find any new occurances of both the IndicesRequestCacheIT.testQueryRewrite and the testQueryRewriteDatesWithNow case since Sep 25th. |
Have a new instance of this: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+intake/255/console
|
We have a couple more errors this weekend:
|
Not This does NOT reproduce locally on me: Log: |
Another example just came up on 6.6. Link to the build: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+6.6+periodic/225 Command to reproduce:
|
Haven't been able to replicate this yet, but had a thought: could time of test run be affecting the Perhaps the |
I have done some investigations for On 6.x it fails from time to time, and Colin's PR to use On master (after Colin's change) it fails very very occasionally. Last failure was in Nov, 2018.
The second miss was supposed to be |
We had another failure last night, this time in a different test method on the same class:
Link to build: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+internalClusterTest/1678/console Reproduce line (doesn't reproduce, of course):
|
Relevant part of the logs for reader versions:
|
The forcemerge request before we start sending search requests should be blocking until it completes. My only thoughts on how we could still end up with a refresh after this is:
|
Looks like we had another instance of it in https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+6.8+matrix-java-periodic/ES_BUILD_JAVA=openjdk12,ES_RUNTIME_JAVA=adoptopenjdk8,nodes=general-purpose/256/console
|
This test failed again today: https://gradle-enterprise.elastic.co/s/zbfbykxpdnfye with the error:
Because this test is failing regularly on master so I muted it in de4cf2b. |
It also fails on 7.x https://gradle-enterprise.elastic.co/s/tkfitlc3lhwmw
I'll wait for one more failure before muting on 7.x as well. |
Another time, and also
I'm going to mute this in both master and 7.x, please feel free to revert if I'm being too aggressive. |
This adds reenables IndicesRequestCacheIT.testQueryRewrite and enables logging for it. Relates to elastic#32827
This adds reenables IndicesRequestCacheIT.testQueryRewrite and enables logging for it. Relates to #32827
Muted |
Analyzing console output from this build: https://gradle-enterprise.elastic.co/s/2gsqxhzsvwdjq
Shard: [index][0] – doesn't have any docs Shard: [index][1] – doesn't have any docs Shard: [index][2] – has docs 2016-03-22 – 2016-03-24 – match_all Shard: [index][3] – has docs 2016-03-19 – 2016-03-21– match_all Shard: [index][4] – has docs 2016-03-25 – 2016-03-27
Shard: [index][0] Shard: [index][1] Shard: [index][2] – has docs 2016-03-22 – 2016-03-24 – match_all Shard: [index][3] – –has docs 2016-03-19 – 2016-03-21– Shard: [index][4] – has docs 2016-03-25 – 2016-03-27 Shard: [index][2] as it rewrites to match_all for both queries should be a hit in the second time, but it was a miss. Looks like cache doesn't work well |
from another build: https://gradle-enterprise.elastic.co/s/fhw4lkpjv5htu expected miss for the 1st time[node_s0] Cache miss for reader version [10], max_doc[5] and request: expected hit for the 2nd time[node_s0] Cache hit for reader version [10], max_doc[5] and request: unexpected miss for the 3rd time[node_s0] Cache miss for reader version [10], max_doc[5] and request: |
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
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 #32827
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
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
…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
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
IndicesRequestCacheIT#testQueryRewrite fails with an unexpected cache count.
This does not reproduce locally for me.
CI: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+periodic/6845/console
Log: testQueryRewrite.txt
The text was updated successfully, but these errors were encountered: