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

Wait on AsynchronousSearchStatsRequest before asserting on statsResponse #617

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

finnegancarroll
Copy link
Collaborator

@finnegancarroll finnegancarroll commented Sep 9, 2024

Description

testStatsAcrossNodes is flaky and I suspect this is because we do not wait on the result of AsynchronousSearchStatsRequest before asserting that node stats equal their expected values. Integration tests fail with this error and while a stack trace is not available it's suspiciously similar to the failure we see when we artificially ensure statsResponse is empty.

  2> REPRODUCE WITH: ./gradlew ':integTest' --tests "org.opensearch.search.asynchronous.integTests.AsynchronousSearchStatsIT.testStatsAcrossNodes" -Dtests.seed=53E00013C44B3A81 -Dtests.security.manager=false -Dtests.locale=en-BM -Dtests.timezone=Australia/Canberra -Druntime.java=21
  2> java.lang.AssertionError: expected:<20> but was:<0>
        at __randomizedtesting.SeedInfo.seed([53E00013C44B3A81:6D05F57FB15B73CC]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:647)
        at org.junit.Assert.assertEquals(Assert.java:633)
        at org.opensearch.search.asynchronous.integTests.AsynchronousSearchStatsIT.testStatsAcrossNodes(AsynchronousSearchStatsIT.java:192)
  2> NOTE: leaving temporary files on disk at: /local/home/carrofin/repos/asynchronous-search/build/testrun/integTest/temp/org.opensearch.search.asynchronous.integTests.AsynchronousSearchStatsIT_53E00013C44B3A81-001
  2> NOTE: test params are: codec=Asserting(Lucene99): {field1.keyword=PostingsFormat(name=Asserting), _routing=PostingsFormat(name=Asserting), index_uuid=PostingsFormat(name=Asserting), field1=PostingsFormat(name=Asserting), _field_names=PostingsFormat(name=Asserting), _id=PostingsFormat(name=Asserting), type=PostingsFormat(name=Asserting)}, docValues:{field1.keyword=DocValuesFormat(name=Asserting), __soft_deletes=DocValuesFormat(name=Asserting), _seq_no=DocValuesFormat(name=Asserting), _tombstone=DocValuesFormat(name=Asserting), _primary_term=DocValuesFormat(name=Asserting), _version=DocValuesFormat(name=Asserting)}, maxPointsInLeafNode=368, maxMBSortInHeap=7.311061141363389, sim=Asserting(RandomSimilarity(queryNorm=true): {}), locale=en-BM, timezone=Australia/Canberra
  2> NOTE: Linux 5.10.224-190.876.amzn2int.x86_64 amd64/Amazon.com Inc. 21.0.4 (64-bit)/cpus=8,threads=2,free=420478976,total=536870912
  2> NOTE: All tests run in this JVM: [AsynchronousSearchStatsIT] 

NOTE: I was NOT able to reproduce this test failure to confirm the fix.

Related Issues

Speculative fix for #87

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.

mch2
mch2 previously approved these changes Sep 10, 2024
@mch2
Copy link
Member

mch2 commented Sep 17, 2024

@finnegancarroll can you pls rebase this to pick up the bwc fixes? Thanks

@mch2 mch2 merged commit a4cf835 into opensearch-project:main Sep 17, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants