Skip to content

Increase await timeout for testCancelFailedSearchWhenPartialResultDisallowed #123084

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.

Sign up for GitHub

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

Jump to bottom
Merged
smalyshev merged 1 commit into from
Feb 25, 2025
Merged

Increase await timeout for testCancelFailedSearchWhenPartialResultDisallowed #123084

smalyshev merged 1 commit into from
Feb 25, 2025

Conversation

@smalyshev
Copy link
Contributor

@smalyshev smalyshev commented Feb 20, 2025 •
edited
Loading

Kinda hard to figure out why it may fail but my one assumption is the search doesn't start for too long so by the time it arrives to latch unlock, it's too late, because assertBusy is 30s, but latch is just 10s.

See: #121719

@smalyshev smalyshev requested a review from JVerwolf February 20, 2025 23:56
@smalyshev smalyshev marked this pull request as ready for review February 20, 2025 23:56
@smalyshev smalyshev added >test Issues or PRs that are addressing/adding tests :Search Foundations/Search Catch all for Search Foundations auto-backport Automatically create backport pull requests when merged v8.19.0 labels Feb 20, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Feb 20, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Feb 20, 2025

Pinging @elastic/es-search-foundations (Team:Search Foundations)

JVerwolf
JVerwolf reviewed Feb 24, 2025
View reviewed changes
Copy link
Contributor

@JVerwolf JVerwolf left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This seems unlikely to be the cause, for the same reasons previously mentioned by @armin here.

@smalyshev
Copy link
Contributor Author

smalyshev commented Feb 24, 2025

@JVerwolf There are different failures on that test, so while it's not the cause, it seems to cause some secondary failures. Not sure it it's worth fixing or fixing the main cause would also fix this...

JVerwolf
JVerwolf approved these changes Feb 24, 2025
View reviewed changes
Copy link
Contributor

@JVerwolf JVerwolf left a comment

Choose a reason for hiding this comment

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

Thanks for the context @smalyshev. Ok, I'll approve this then.

WDYT about this issue marked as fixing #121719 - this isn't quite true, right?

@smalyshev
Copy link
Contributor Author

smalyshev commented Feb 24, 2025

No, it's totally not true, I meant it as reference, and wrote "fixes" out of habit, it shouldn't actually close that issue.

@smalyshev smalyshev merged commit 688542b into elastic:main Feb 25, 2025
17 checks passed
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Feb 25, 2025

💔 Backport failed

Status Branch Result
❌ 8.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 123084

@smalyshev smalyshev mentioned this pull request Feb 25, 2025
Merged
@smalyshev
Copy link
Contributor Author

smalyshev commented Feb 25, 2025

💚 All backports created successfully

Status Branch Result
✅ 8.x

Questions ?

Please refer to the Backport tool documentation

elasticsearchmachine pushed a commit that referenced this pull request Feb 25, 2025
@smalyshev
Increase await timeout for testCancelFailedSearchWhenPartialResultDis…
7935439 
…allowed (#123084) (#123335)

(cherry picked from commit 688542b)

# Conflicts:
#	muted-tests.yml
@ywangd
Copy link
Member

ywangd commented Feb 27, 2025

The same failure still happened with the change here. See https://gradle-enterprise.elastic.co/s/oau6o5i3yjwua

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@original-brownbear original-brownbear Awaiting requested review from original-brownbear

1 more reviewer

@JVerwolf JVerwolf JVerwolf approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

auto-backport Automatically create backport pull requests when merged :Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch >test Issues or PRs that are addressing/adding tests v8.19.0 v9.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@smalyshev @elasticsearchmachine @ywangd @JVerwolf