Skip to content

fix composite aggregation tests failing after #76740#77691

Merged
Luegg merged 5 commits intoelastic:masterfrom
Luegg:fix/compositeAggs
Sep 15, 2021
Merged

fix composite aggregation tests failing after #76740#77691
Luegg merged 5 commits intoelastic:masterfrom
Luegg:fix/compositeAggs

Conversation

@Luegg
Copy link
Copy Markdown
Contributor

@Luegg Luegg commented Sep 14, 2021

Follow up of #77650.

Looks like two issues caused occasional failures:

  • missingOrder has not been considered in InternalComposite.InternalBucket#compareKey
  • reverseMul has been ignored in one case in OrdinalValuesSource#compareInternal

The first problem has clearly been introduced by #76740 but the second point is more interesting. As far as I understand, this might have caused problems before but I couldn't find any according test failures in Gradle before yesterday.

In order to preclude further rare test failures I've run the whole CompositeAggregatorTests 3000 times and all tests turned green.

@Luegg
Copy link
Copy Markdown
Contributor Author

Luegg commented Sep 14, 2021

@elasticmachine run elasticsearch-ci/part-1

@Luegg Luegg marked this pull request as ready for review September 14, 2021 14:47
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Sep 14, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@Luegg
Copy link
Copy Markdown
Contributor Author

Luegg commented Sep 14, 2021

Unfortunately, bwc checks are currently failing (#77711) but maybe you already want to have a look.

Copy link
Copy Markdown
Member

@not-napoleon not-napoleon left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for jumping on this!

Copy link
Copy Markdown
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

LGTM. Good luck with the backport dance. That takes a while.

@Luegg Luegg merged commit 7ae4f35 into elastic:master Sep 15, 2021
@Luegg Luegg deleted the fix/compositeAggs branch September 15, 2021 13:31
Luegg pushed a commit to Luegg/elasticsearch that referenced this pull request Sep 15, 2021
…77691)

Follow up of elastic#77650.

Looks like two issues caused occasional failures:

missingOrder has not been considered in InternalComposite.InternalBucket#compareKey
reverseMul has been ignored in one case in OrdinalValuesSource#compareInternal
The first problem has clearly been introduced by elastic#76740 but the second point is more interesting. As far as I understand, this might have caused problems before but I couldn't find any according test failures in Gradle before yesterday.

In order to preclude further rare test failures I've run the whole CompositeAggregatorTests 3000 times and all tests turned green.
# Conflicts:
#	server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/InternalComposite.java
Luegg pushed a commit to Luegg/elasticsearch that referenced this pull request Sep 15, 2021
elasticsearchmachine pushed a commit that referenced this pull request Sep 15, 2021
Follow up of #77650.

Looks like two issues caused occasional failures:

missingOrder has not been considered in InternalComposite.InternalBucket#compareKey
reverseMul has been ignored in one case in OrdinalValuesSource#compareInternal
The first problem has clearly been introduced by #76740 but the second point is more interesting. As far as I understand, this might have caused problems before but I couldn't find any according test failures in Gradle before yesterday.

In order to preclude further rare test failures I've run the whole CompositeAggregatorTests 3000 times and all tests turned green.
# Conflicts:
#	server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/InternalComposite.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v7.16.0 v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants