-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Fix Flaky Semantic Search CCS Integration Tests #135629
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
Fix Flaky Semantic Search CCS Integration Tests #135629
Conversation
|
Pinging @elastic/search-relevance (Team:Search - Relevance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
...lusterTest/java/org/elasticsearch/search/ccs/AbstractSemanticCrossClusterSearchTestCase.java
Outdated
Show resolved
Hide resolved
| .cluster() | ||
| .prepareHealth(TEST_REQUEST_TIMEOUT, indexName) | ||
| .setWaitForYellowStatus() | ||
| .setWaitForGreenStatus() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should never be yellow if there are 0 replicas, only green, so maybe you could keep the wait for yellow status? But I'm OK leaving this as is as long as we don't think there's a testing hole
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, but if it should never be yellow then this check should be fine and IMO at the very least it clarifies the state we want the clusters to be in before starting tests.
| RemoteInfoRequest request = new RemoteInfoRequest(); | ||
| boolean connected; | ||
| int attempts = 0; | ||
| int delayInSeconds = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be made simpler reusing assertBusy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in f07145f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes flaky semantic search CCS integration tests by making the following changes:
Fixes #135559 and #135573