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

Fixed flaky hybrid collector test #984

Merged

Conversation

owaiskazi19
Copy link
Member

@owaiskazi19 owaiskazi19 commented Nov 13, 2024

Description

Fixed flakiness of testReduceWithConcurrentSegmentSearch_whenMultipleCollectorsMatchedDocs_thenSuccessful

Fix:

  1. Ensured consistent document count: Instead of using RandomizedTest.randomInt() for document IDs, use fixed values or a sequence to ensure a consistent number of documents.

  2. Control the number of documents: Add a fixed number of documents to each index, ensuring it's always greater than the number expecting in the results.

  3. Adjust the search size: Made sure the search size (when(searchContext.size()).thenReturn(1)) is consistent with the number of documents adding and expecting in the results.

Ran test 100 times, worked fine with the change

 ./gradlew ':test' --tests "org.opensearch.neuralsearch.search.query.HybridCollectorManagerTests.testReduceWithConcurrentSegmentSearch_whenMultipleCollectorsMatchedDocs_thenSuccessful" -Dtests.seed=8305B866B5D072CE -Dtests.security.manager=false -Dtests.locale=af-ZA -Dtests.timezone=Europe/Uzhgorod -Dtests.iters=100
=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 8.9
  OS Info               : Mac OS X 14.7 (aarch64)
  JDK Version           : 21 (Oracle JDK)
  JAVA_HOME             : /Library/Java/JavaVirtualMachines/jdk-21.jdk/Contents/Home
  Random Testing Seed   : 8305B866B5D072CE
  In FIPS 140 mode      : false
=======================================

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.9/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 1s

Related Issues

Resolves #819

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@martin-gaievski
Copy link
Member

Can you describe at high level what was the issue and what's the leading idea for this fix?

@martin-gaievski
Copy link
Member

martin-gaievski commented Nov 13, 2024

Does the 2.x branch have same problem? If so we need to add backport 2.x label

@owaiskazi19
Copy link
Member Author

Does the 2.x branch have same problem? If so we need to add backport 2.x label

Yes, I see failure on 2.x. We should backport this one

org.opensearch.neuralsearch.search.query.HybridCollectorManagerTests > testReduceWithConcurrentSegmentSearch_whenMultipleCollectorsMatchedDocs_thenSuccessful {seed=[8305B866B5D072CE:E498F451C7B14F98]} FAILED
    java.lang.IllegalArgumentException: cannot merge top docs because it does not have enough elements
        at __randomizedtesting.SeedInfo.seed([8305B866B5D072CE:E498F451C7B14F98]:0)
        at org.opensearch.neuralsearch.search.query.HybridQueryScoreDocsMerger.merge(HybridQueryScoreDocsMerger.java:42)
        at org.opensearch.neuralsearch.search.query.TopDocsMerger.getMergedScoreDocs(TopDocsMerger.java:130)
        at org.opensearch.neuralsearch.search.query.TopDocsMerger.merge(TopDocsMerger.java:69)
        at org.opensearch.neuralsearch.search.query.HybridCollectorManager.reduceCollectorResults(HybridCollectorManager.java:446)
        at org.opensearch.neuralsearch.search.query.HybridCollectorManager.lambda$getSearchResults$0(HybridCollectorManager.java:169)
        at org.opensearch.neuralsearch.search.query.HybridCollectorManager.lambda$reduceSearchResults$11(HybridCollectorManager.java:459)
        at org.opensearch.neuralsearch.search.query.HybridCollectorManagerTests.testReduceWithConcurrentSegmentSearch_whenMultipleCollectorsMatchedDocs_thenSuccessful(HybridCollectorManagerTests.java:598)


@owaiskazi19
Copy link
Member Author

owaiskazi19 commented Nov 13, 2024

Can you describe at high level what was the issue and what's the leading idea for this fix?

Updated the description

@martin-gaievski martin-gaievski added the backport 2.x Label will add auto workflow to backport PR to 2.x branch label Nov 13, 2024
@martin-gaievski
Copy link
Member

Looks good to me, thank you

@vibrantvarun vibrantvarun merged commit 9412884 into opensearch-project:main Nov 13, 2024
43 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 13, 2024
* Fixed flaky hybrid collector test

Signed-off-by: Owais <[email protected]>

* Removed explicit exception

Signed-off-by: Owais <[email protected]>

---------

Signed-off-by: Owais <[email protected]>
(cherry picked from commit 9412884)
vibrantvarun pushed a commit that referenced this pull request Nov 13, 2024
* Fixed flaky hybrid collector test

Signed-off-by: Owais <[email protected]>

* Removed explicit exception

Signed-off-by: Owais <[email protected]>

---------

Signed-off-by: Owais <[email protected]>
(cherry picked from commit 9412884)

Co-authored-by: Owais Kazi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Label will add auto workflow to backport PR to 2.x branch flaky-test good first issue Good for newcomers skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Flaky test: testReduceWithConcurrentSegmentSearch_whenMultipleCollectorsMatchedDocs_thenSuccessful
3 participants