-
Notifications
You must be signed in to change notification settings - Fork 25k
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
[CI] MixedClusterClientYamlTestSuiteIT test {p0=search.vectors/60_dense_vector_dynamic_mapping/Fields with float arrays within the threshold map as dense_vector} failing #100502
Comments
Pinging @elastic/es-search (Team:Search) |
I cannot get this to replicate locally :(. I even tried going back to the particular versions of the upgraded and old servers. From what I can tell, every time we create th For this particular failure, I did notice that the shard becomes GREEN between us applying the mapping and then updating it.
But that shouldn't be a problem as we already call with |
Adding some logging via: #100546 I see the mapping gets updated, but then the API complains that the Just wanting to be 100% sure the mapping is being updated correctly in this mix cluster scenario via logging. |
In my PR to add more logging, a similar test failed & had my higher logging fidelity:
We can see that the
|
I have updated the logging for this test via: #100546 if it fails again after that commit, we should be able to make a better diagnosis. |
Here you have: https://gradle-enterprise.elastic.co/s/3ksaph7noy32y |
@iverase my PR isn't merged yet :( hopefully it will fail after. Or better yet, during the CI build for the PR! |
This adds a test for dynamic dims update mapping merges. Also, this adds logging to help investigate a periodically failing test. related to: #100502
FYI person who sees this test fail on their PR. Please let me know the build so I can verify my logging is there. Once I have the logging, I can see about muting the test myself if it is a nuisance. |
…00546) This adds a test for dynamic dims update mapping merges. Also, this adds logging to help investigate a periodically failing test. related to: elastic#100502
Failed in build: https://gradle-enterprise.elastic.co/s/db27w6u4cwpck Here are the logs, they contain my trace logging. |
On
So, we are obviously updating the mapping. I wonder if the |
With the additional logging: https://gradle-enterprise.elastic.co/s/4cgo6n7lqhbki Test failed around
It SEEMS like the new mapping is in cluster state version: Is this a correct reading of |
Not quite I think. The exception in that failure is |
Oh I think I see:
We apply the mapping update on |
I can try to add these to our yaml tests. |
I think we have a handle on the problem and its solution here now so I'll mute the tests. |
…101006) I noticed that sometimes we read from the `fieldType()` and other times we read from the mapper directly. It seems to me that we should only ever read and update values from one of those, for sanity's sake. So, I removed all values that were part of the mapper directly and used `fieldType().<value>` everywhere. Additionally, David Turner suggested that we wait for cluster health before verifying mappings in the yaml tests, so I added that as well. related to: #100502
…lastic#101006) I noticed that sometimes we read from the `fieldType()` and other times we read from the mapper directly. It seems to me that we should only ever read and update values from one of those, for sanity's sake. So, I removed all values that were part of the mapper directly and used `fieldType().<value>` everywhere. Additionally, David Turner suggested that we wait for cluster health before verifying mappings in the yaml tests, so I added that as well. related to: elastic#100502
…101006) (#101076) I noticed that sometimes we read from the `fieldType()` and other times we read from the mapper directly. It seems to me that we should only ever read and update values from one of those, for sanity's sake. So, I removed all values that were part of the mapper directly and used `fieldType().<value>` everywhere. Additionally, David Turner suggested that we wait for cluster health before verifying mappings in the yaml tests, so I added that as well. related to: #100502
These were failing pretty much at least once a day for weeks, I have verified since merging the fix, these having failed. I am going to revert all the logging changes and close this issue. |
Test hasn't failed for over a week, the original issue seems resolved as the at least one of the tests were failing once a day. closes elastic#100502 (cherry picked from commit 3b0b484)
Test has failed just the once.
Build scan:
https://gradle-enterprise.elastic.co/s/bdc4arhkiazly/tests/:qa:mixed-cluster:v8.11.0%23mixedClusterTest/org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT/test%20%7Bp0=search.vectors%2F60_dense_vector_dynamic_mapping%2FFields%20with%20float%20arrays%20within%20the%20threshold%20map%20as%20dense_vector%7D
Reproduction line:
Applicable branches:
main
Reproduces locally?:
No
Failure history:
https://gradle-enterprise.elastic.co/scans/tests?tests.container=org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT&tests.test=test%20%7Bp0%3Dsearch.vectors/60_dense_vector_dynamic_mapping/Fields%20with%20float%20arrays%20within%20the%20threshold%20map%20as%20dense_vector%7D
Failure excerpt:
The text was updated successfully, but these errors were encountered: