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

Exchange should wait for remote sinks #108337

Merged
merged 1 commit into from
May 8, 2024

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented May 7, 2024

Today, we do not wait for remote sinks to stop before completing the main request. While this doesn't affect correctness, it's important to note that we should never spawn child requests after the parent request is completed.

Closes #105859

@dnhatn dnhatn added v8.14.1 v8.13.5 >non-issue auto-backport-and-merge Automatically create backport pull requests and merge when ready :Analytics/ES|QL AKA ESQL labels May 8, 2024
@dnhatn dnhatn requested a review from nik9000 May 8, 2024 17:47
@dnhatn dnhatn marked this pull request as ready for review May 8, 2024 17:47
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 8, 2024
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

That does seem more correct.

@dnhatn
Copy link
Member Author

dnhatn commented May 8, 2024

Thanks Nik!

@dnhatn dnhatn merged commit ab40808 into elastic:main May 8, 2024
15 checks passed
@dnhatn dnhatn deleted the wait-for-remote-sinks branch May 8, 2024 18:32
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request May 8, 2024
Today, we do not wait for remote sinks to stop before completing the 
main request. While this doesn't affect correctness, it's important  that
we do not spawn child requests after the parent request is completed.

Closes elastic#105859
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented May 8, 2024

💔 Backport failed

Status Branch Result
8.14

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

dnhatn added a commit that referenced this pull request May 8, 2024
Today, we do not wait for remote sinks to stop before completing the 
main request. While this doesn't affect correctness, it's important  that
we do not spawn child requests after the parent request is completed.

Closes #105859
markjhoy pushed a commit to markjhoy/elasticsearch that referenced this pull request May 9, 2024
Today, we do not wait for remote sinks to stop before completing the 
main request. While this doesn't affect correctness, it's important  that
we do not spawn child requests after the parent request is completed.

Closes elastic#105859
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request May 20, 2024
Today, we do not wait for remote sinks to stop before completing the
main request. While this doesn't affect correctness, it's important  that
we do not spawn child requests after the parent request is completed.

Closes elastic#105859
@dnhatn dnhatn added the v8.13.5 label May 20, 2024
elasticsearchmachine pushed a commit that referenced this pull request May 20, 2024
Today, we do not wait for remote sinks to stop before completing the
main request. While this doesn't affect correctness, it's important  that
we do not spawn child requests after the parent request is completed.

Closes #105859
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-backport-and-merge Automatically create backport pull requests and merge when ready >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.13.5 v8.14.1 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] ExchangeServiceTests testFailToRespondPage failing
3 participants