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

Fix DataNodeRequestSender #121999

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix DataNodeRequestSender #121999

wants to merge 1 commit into from

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Feb 7, 2025

There are two issues in the current implementation:

  1. We should use the list of shardIds from the request, rather than all targets, when removing failures for shards that have been successfully executed.

  2. We should remove shardIds from the pending list once a failure is reported and abort execution at that point, as the results will be discarded.

Closes #121966

@dnhatn dnhatn marked this pull request as ready for review February 8, 2025 01:48
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Feb 8, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] DataNodeRequestSenderTests testDoNotRetryOnRequestLevelFailure failing
2 participants