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

increase cluster node timeout value for time consuming test cases #1060

Conversation

will-hwang
Copy link
Contributor

@will-hwang will-hwang commented Jan 5, 2025

Description

This change introduces a refactor of the code in BaseNeuralSearchIT to enable flexible timeout setting for cluster nodes, and increases the time out value from 60s to 90s for the below test cases:

testBatchIngestion_SparseEncodingProcessor_E2EFlow
testTextImageEmbeddingProcessor_E2EFlow
testSparseEncodingProcessor_E2EFlow
testSemanticSearch_E2EFlow

Based on the previous PRs (link), these test cases in BWC suite frequently failed with timeout, which requires manual re-run of the CI action

Related Issues

Resolves #1035 #981

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.

waitForGreen.addParameter("cluster_manager_timeout", "60s");
waitForGreen.addParameter("timeout", "60s");
waitForGreen.addParameter("cluster_manager_timeout", String.format(LOCALE, "%ds", timeoutInSeconds));
waitForGreen.addParameter("timeout", String.format(LOCALE, "%ds", timeoutInSeconds));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
waitForGreen.addParameter("timeout", String.format(LOCALE, "%ds", timeoutInSeconds));
waitForGreen.addParameter("timeout", String.format(Locale.ROOT, "%ds", timeoutInSeconds));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like both LOCALE variable is initialized here, but both LOCALE and Local.ROOT are used in different places in the file. should we make the change in other places as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If they are same, I am fine. Thanks!

@heemin32 heemin32 added skip-changelog backport 2.x Label will add auto workflow to backport PR to 2.x branch and removed bug Something isn't working labels Jan 5, 2025
@zhichao-aws
Copy link
Member

Can we change the target branch to main instead of 2.x? The recommended way is commit to main and backport to 2.x

@martin-gaievski martin-gaievski changed the base branch from 2.x to main January 6, 2025 16:27
@github-actions github-actions bot added the bug Something isn't working label Jan 6, 2025
@martin-gaievski martin-gaievski changed the base branch from main to 2.x January 6, 2025 16:28
@martin-gaievski
Copy link
Member

martin-gaievski commented Jan 6, 2025

Can we change the target branch to main instead of 2.x? The recommended way is commit to main and backport to 2.x

@will-hwang You need to create branch for this PR based on main branch. I've tried to simply change the base to main and GH gives me 300+ changes, so such approach is not practical.

@will-hwang
Copy link
Contributor Author

closing this PR since i've opened up a new PR into main branch

@will-hwang will-hwang closed this Jan 6, 2025
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 bug Something isn't working skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FlakyTest] org.opensearch.neuralsearch.bwc.SemanticSearchIT.testSemanticSearch_E2EFlow
4 participants