Skip to content

Conversation

@cbuescher
Copy link
Member

The referenced issue #40005 is marked as closed, so we should either be able to remove the @AwaitsFix notation and be able to run the test or we should create another issue in case the test still fails.

@cbuescher cbuescher added >test Issues or PRs that are addressing/adding tests :Search/Search Search-related issues that do not fall into other categories labels Mar 31, 2022
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Mar 31, 2022
@elasticmachine
Copy link
Collaborator

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

@cbuescher cbuescher added v8.3.0 v8.2.1 and removed Team:Search Meta label for search team labels Mar 31, 2022
@cbuescher cbuescher self-assigned this Mar 31, 2022
@cbuescher cbuescher force-pushed the remove-awaits-fix-40005 branch from 2f9a3f5 to 6a638e4 Compare March 31, 2022 10:26
@cbuescher
Copy link
Member Author

So this currently still fails on some equality assertion that only seems to be caused by two object instances not being the same (they use Object#toString() apparently in the output). I wonder if that is just something we have to relax in the test?

name: 
expected String [MultiBucketCollector: [[org.elasticsearch.search.aggregations.bucket.terms.StringTermsAggregatorFromFilters@2eb2f8f0, org.elasticsearch.search.aggregations.bucket.terms.StringTermsAggregatorFromFilters@44e681ed, answers]]] 
 but was String [MultiBucketCollector: [[org.elasticsearch.search.aggregations.bucket.terms.StringTermsAggregatorFromFilters@abf7402, org.elasticsearch.search.aggregations.bucket.terms.StringTermsAggregatorFromFilters@32801d7b, answers]]]

@javanna
Copy link
Member

javanna commented Mar 31, 2022

thanks for opening this @cbuescher . Is it clear how these two objects differ? I would probably print out their xcontent to try and see how to move forward.

@cbuescher
Copy link
Member Author

Is it clear how these two objects differ? I would probably print out their xcontent to try and see how to move forward.

Yes, the above is already the output of the json resonse (expected vs. actual) that the test graciously reports. As far as I can see the value of the "name" field differs in its String representation. I was going to check why (or why we even use the raw objects "toString" as in org.elasticsearch.search.aggregations.bucket.terms.StringTermsAggregatorFromFilters@2eb2f8f0) here in the first place.

@cbuescher
Copy link
Member Author

So it appears that "profile" uses Aggregators "toString()" method, which e.g. AggregatorBase implements by simply printing the name of the aggregation, but the offending one above extends AdaptingAggregator which doesn't overwrite Object#toString(). I added that now to see what happens next.

@cbuescher
Copy link
Member Author

Locally this passed now, but a specific seed surfaced another error, but this might turn out to be a genuine test error, I'm not sure though:

./gradlew ':qa:multi-cluster-search:mixedClusterTest' --tests "org.elasticsearch.search.CCSDuelIT.testTermsAggsWithProfile" -Dtests.seed=E361034F6E6B9E7A

@javanna
Copy link
Member

javanna commented Apr 1, 2022

Can you post the error @cbuescher ?

@cbuescher
Copy link
Member Author

Some bucket counts for what seems to be the terms aggregation are different between the two responses that are being compared by this test, i.e.

org.elasticsearch.search.CCSDuelIT > testTermsAggsWithProfile FAILED
    java.lang.AssertionError: Didn't match expected value:
                         _clusters: 
                             skipped: same [0]
                          successful: same [2]
                               total: same [2]
                           _shards: 
                              failed: same [0]
                          successful: same [5]
                               total: same [5]
                      aggregations: 
                      filter#answers: 
                             doc_count: same [250]
            sterms#answer_per_question: 
                                 buckets: 
                                         0: 
                                   doc_count: same [6]
                 doc_count_error_upper_bound: same [0]
                                         key: same [multi_cluster-60]
                                         1: 
                                   doc_count: same [5]
                 doc_count_error_upper_bound: expected Integer [2] but was Integer [0]
                                         key: same [remote_cluster-93]
                                         2: 
                                   doc_count: same [4]
                 doc_count_error_upper_bound: same [0]
                                         key: same [multi_cluster-73]
                                         3: 
                                   doc_count: same [4]
                 doc_count_error_upper_bound: expected Integer [2] but was Integer [0]
                                         key: same [remote_cluster-18]
                                         4: 
                                   doc_count: same [4]
                 doc_count_error_upper_bound: expected Integer [2] but was Integer [0]
                                         key: same [remote_cluster-2]
                                         5: 
                                   doc_count: same [4]
                 doc_count_error_upper_bound: expected Integer [2] but was Integer [0]
                                         key: same [remote_cluster-22]
                                         6: 
                                   doc_count: same [4]
                 doc_count_error_upper_bound: expected Integer [2] but was Integer [0]
                                         key: same [remote_cluster-47]
                                         7: 
                                   doc_count: same [4]
                 doc_count_error_upper_bound: expected Integer [2] but was Integer [0]
                                         key: same [remote_cluster-52]
                                         8: 
                                   doc_count: same [4]
                 doc_count_error_upper_bound: expected Integer [2] but was Integer [0]
                                         key: same [remote_cluster-77]
                                         9: 
                                   doc_count: same [4]
                 doc_count_error_upper_bound: expected Integer [2] but was Integer [0]
                                         key: same [remote_cluster-94]
             doc_count_error_upper_bound: expected Integer [2] but was Integer [0]
                     sum_other_doc_count: same [207]

Full context can be seem here

Maybe we should open a new issue for investigation around this and replace the existing @AwaitsFix issue number with that issue?

@cbuescher
Copy link
Member Author

Re-checked this after merging #85281. The reproduce line slightly changed due to the changes in the gradle build file. The error with the unexpected values in "doc_count_error_upper_bound" now shows e.g. with

./gradlew ':qa:multi-cluster-search:v8.3.0#multi-cluster' -Dtests.class="org.elasticsearch.search.CCSDuelIT" -Dtests.method="testTermsAggsWithProfile" -Dtests.seed=FC39998A856BD0CF

@javanna
Copy link
Member

javanna commented Apr 20, 2022

I am wondering if this is another occurrence of the same bug that we thought was fixed, or rather a test bug. I am moving this to the Analytics team to check out.

@javanna javanna added :Analytics/Aggregations Aggregations and removed :Search/Search Search-related issues that do not fall into other categories labels Apr 20, 2022
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Apr 20, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@cbuescher
Copy link
Member Author

I think it makes sense to split out the one-liner change in 860de2c that otherwise hides the mismatch in "doc_count_error_upper_bound" in a separate PR

@cbuescher
Copy link
Member Author

@elasticmachine test this please

@cbuescher
Copy link
Member Author

I'm closing this PR but re-opened #40005 for tracking, since I think having an open issue instead of a PR increases its visibility.

@cbuescher cbuescher closed this Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Aggregations Aggregations Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v8.2.1 v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants