Dart: Wait for a merge buffer if none is available.#18075
Merged
Conversation
For Dart-only workloads, it is not possible for merge buffers to be exhausted, because the number of concurrent queries is capped at the number of merge buffers by DartWorkerMemoryManagementModule. However, When native and Dart queries are run on the same Historical, it is possible for buffers to be held by native queries. In this case, Dart workers must wait for buffers to become available. This waiting now happens in the worker runner thread, and lasts for the configured query timeout. At this time, the waiting is not interrupted when a cancellation request arrives from the controller. This should be addressed in another patch.
gianm
force-pushed
the
msq-dart-await-buffer
branch
from
June 4, 2025 06:08
d3b60b1 to
eecf2c6
Compare
gianm
added a commit
to gianm/druid
that referenced
this pull request
Jun 8, 2025
This patch simplifies the worker lifecycle and focuses on using interruption instead of various flags to manage cancellation. This should enable workers to cancel themselves more quickly, and is particularly important when they may be waiting for a resource to become available (such as merge buffers as in apache#18075). Changes: 1) Introduce WorkerRunRef to manage worker execution and cancellation. This class encapsulates the logic for running a worker in an executor and canceling it via thread interruption. 2) Move periodic controller status checking out of IndexerWorkerContext and into a dedicated PeriodicControllerChecker class. 3) Remove stop(), awaitStop(), and controllerFailed() from the Worker interface. Workers are now canceled via interruption only. 4) Remove controllerAlive tracking from WorkerImpl. It should no longer be necessary now that workers are interrupted on controller failure.
clintropolis
approved these changes
Jun 11, 2025
kgyrtkirk
reviewed
Jun 12, 2025
| * Acquire buffers for a {@link Worker}. | ||
| */ | ||
| ResourceHolder<ProcessingBuffersSet> acquire(int poolSize); | ||
| ResourceHolder<ProcessingBuffersSet> acquire(int poolSize, long timeoutMillis); |
Member
There was a problem hiding this comment.
note: I think the usage of Duration would be usefull
Contributor
Author
There was a problem hiding this comment.
IMO a long makes sense because there's only one call site, and it passes queryDef.getContext().getTimeout() which is a long.
Comment on lines
+62
to
+64
| /** | ||
| * Acquire buffers if a particular stages needs them; otherwise, returns null. | ||
| */ |
Member
There was a problem hiding this comment.
note: instead of this apidoc + @Nullable annotations and the if-s handling the null: it would be usefull to return a ResourceHolder which's get throws the exception that there is no buffer
Contributor
Author
There was a problem hiding this comment.
Fair enough, I made this change.
jtuglu1
pushed a commit
to jtuglu1/druid
that referenced
this pull request
Jun 17, 2025
* Dart: Wait for a merge buffer if none is available. For Dart-only workloads, it is not possible for merge buffers to be exhausted, because the number of concurrent queries is capped at the number of merge buffers by DartWorkerMemoryManagementModule. However, When native and Dart queries are run on the same Historical, it is possible for buffers to be held by native queries. In this case, Dart workers must wait for buffers to become available. This waiting now happens in the worker runner thread, and lasts for the configured query timeout. At this time, the waiting is not interrupted when a cancellation request arrives from the controller. This should be addressed in another patch. * Add test coverage for DartProcessingBuffersProvider. * Changes from review. * Add tests. * More tests.
gianm
added a commit
that referenced
this pull request
Jun 27, 2025
* MSQ: Use interrupts for worker cancellation. This patch simplifies the worker lifecycle and focuses on using interruption instead of various flags to manage cancellation. This should enable workers to cancel themselves more quickly, and is particularly important when they may be waiting for a resource to become available (such as merge buffers as in #18075). Changes: 1) Introduce WorkerRunRef to manage worker execution and cancellation. This class encapsulates the logic for running a worker in an executor and canceling it via thread interruption. 2) Move periodic controller status checking out of IndexerWorkerContext and into a dedicated PeriodicControllerChecker class. 3) Remove stop(), awaitStop(), and controllerFailed() from the Worker interface. Workers are now canceled via interruption only. 4) Remove controllerAlive tracking from WorkerImpl. It should no longer be necessary now that workers are interrupted on controller failure. * Style fixes.
riovic918data
pushed a commit
to riovic918data/druid
that referenced
this pull request
Jun 12, 2026
* Dart: Wait for a merge buffer if none is available. For Dart-only workloads, it is not possible for merge buffers to be exhausted, because the number of concurrent queries is capped at the number of merge buffers by DartWorkerMemoryManagementModule. However, When native and Dart queries are run on the same Historical, it is possible for buffers to be held by native queries. In this case, Dart workers must wait for buffers to become available. This waiting now happens in the worker runner thread, and lasts for the configured query timeout. At this time, the waiting is not interrupted when a cancellation request arrives from the controller. This should be addressed in another patch. * Add test coverage for DartProcessingBuffersProvider. * Changes from review. * Add tests. * More tests.
riovic918data
pushed a commit
to riovic918data/druid
that referenced
this pull request
Jun 12, 2026
* MSQ: Use interrupts for worker cancellation. This patch simplifies the worker lifecycle and focuses on using interruption instead of various flags to manage cancellation. This should enable workers to cancel themselves more quickly, and is particularly important when they may be waiting for a resource to become available (such as merge buffers as in apache#18075). Changes: 1) Introduce WorkerRunRef to manage worker execution and cancellation. This class encapsulates the logic for running a worker in an executor and canceling it via thread interruption. 2) Move periodic controller status checking out of IndexerWorkerContext and into a dedicated PeriodicControllerChecker class. 3) Remove stop(), awaitStop(), and controllerFailed() from the Worker interface. Workers are now canceled via interruption only. 4) Remove controllerAlive tracking from WorkerImpl. It should no longer be necessary now that workers are interrupted on controller failure. * Style fixes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For Dart-only workloads, it is not possible for merge buffers to be exhausted, because the number of concurrent queries is capped at the number of merge buffers by DartWorkerMemoryManagementModule. However, When native and Dart queries are run on the same Historical, it is possible for buffers to be held by native queries. In this case, Dart workers must wait for buffers to become available.
This waiting now happens in the worker runner thread, and lasts for the configured query timeout. At this time, the waiting is not interrupted when a cancellation request arrives from the controller. This should be addressed in another patch.