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

Resume Driver on cancelled or early finished #120020

Merged
merged 6 commits into from
Jan 15, 2025
Merged

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Jan 12, 2025

Today, when a Driver is put to sleep or scheduled but the thread pool is busy with other tasks, canceling the Driver must wait until the Driver is awakened and scheduled for another loop, or until the thread pool completes the tasks ahead of it. A similar issue occurs with early finishes.

This change enables preemptive early finishes or cancellations of the Driver without waiting for another run loop.

@dnhatn dnhatn changed the title Resume blocking Driver when any operator isFinished Resume blocking Driver when any operator finished Jan 12, 2025
@dnhatn dnhatn force-pushed the driver-blocking branch 2 times, most recently from 857b697 to f250e18 Compare January 14, 2025 03:57
@dnhatn dnhatn changed the title Resume blocking Driver when any operator finished Resume Driver on cancelled or early finished Jan 14, 2025
@dnhatn dnhatn force-pushed the driver-blocking branch 8 times, most recently from c22faad to 955aa6b Compare January 14, 2025 07:15
@dnhatn dnhatn requested review from nik9000 and smalyshev January 14, 2025 07:15
@elasticsearchmachine
Copy link
Collaborator

Hi @dnhatn, I've created a changelog YAML for you.

@dnhatn dnhatn marked this pull request as ready for review January 14, 2025 07:15
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jan 14, 2025
@dnhatn dnhatn added :Analytics/ES|QL AKA ESQL and removed :Analytics/Compute Engine Analytics in ES|QL labels Jan 14, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @dnhatn, I've created a changelog YAML for you.

This reverts commit 03c65d0100314855345ad16625c9ab543f74a7c2.
@smalyshev
Copy link
Contributor

I tested this against #118122 and it seems to work fine and eliminated the need for setSource in ExchangeSinkHandler.

@dnhatn
Copy link
Member Author

dnhatn commented Jan 15, 2025

Thanks, Stas, for testing this patch!

Thanks, Nik, for the feedback - it makes sense. I've made two changes:

  1. Introduced DriverScheduler, which handles scheduled tasks, delayed tasks, and aborting. This change should simplify the Driver code.

  2. Removed the isFinishedListener, as its purpose seemed unclear. Instead, I used a concrete ExchangeSinkOperator to handle early finish notifications.

@dnhatn dnhatn added the auto-backport Automatically create backport pull requests when merged label Jan 15, 2025
@dnhatn dnhatn merged commit 1f182e7 into elastic:main Jan 15, 2025
16 checks passed
@dnhatn dnhatn deleted the driver-blocking branch January 15, 2025 15:30
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

smalyshev added a commit to smalyshev/elasticsearch that referenced this pull request Jan 15, 2025
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Jan 15, 2025
Today, when a Driver is put to sleep or scheduled but the thread pool is 
busy with other tasks, canceling the Driver must wait until the Driver
is awakened and scheduled for another loop, or until the thread pool
completes the tasks ahead of it. A similar issue occurs with early
finishes.

This change enables preemptive early finishes or cancellations of the 
Driver without waiting for another run loop.
elasticsearchmachine pushed a commit that referenced this pull request Jan 15, 2025
* Resume Driver on cancelled or early finished (#120020)

Today, when a Driver is put to sleep or scheduled but the thread pool is 
busy with other tasks, canceling the Driver must wait until the Driver
is awakened and scheduled for another loop, or until the thread pool
completes the tasks ahead of it. A similar issue occurs with early
finishes.

This change enables preemptive early finishes or cancellations of the 
Driver without waiting for another run loop.

* compile
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 Automatically create backport pull requests when merged >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants