You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "gianm (via GitHub)" <gi...@apache.org> on 2025/06/04 05:57:46 UTC
[PR] Dart: Wait for a merge buffer if none is available. (druid)
gianm opened a new pull request, #18075:
URL: https://github.com/apache/druid/pull/18075
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Dart: Wait for a merge buffer if none is available. (druid)
Posted by "kgyrtkirk (via GitHub)" <gi...@apache.org>.
kgyrtkirk commented on code in PR #18075:
URL: https://github.com/apache/druid/pull/18075#discussion_r2142018331
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ProcessingBuffersProvider.java:
##########
@@ -32,7 +32,7 @@ public interface ProcessingBuffersProvider
/**
* Acquire buffers for a {@link Worker}.
*/
- ResourceHolder<ProcessingBuffersSet> acquire(int poolSize);
+ ResourceHolder<ProcessingBuffersSet> acquire(int poolSize, long timeoutMillis);
Review Comment:
note: I think the usage of `Duration` would be usefull
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ProcessingBuffersSet.java:
##########
@@ -59,13 +59,25 @@ public static <T extends Collection<ByteBuffer>> ProcessingBuffersSet fromCollec
);
}
+ /**
+ * Acquire buffers if a particular stages needs them; otherwise, returns null.
+ */
Review 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Dart: Wait for a merge buffer if none is available. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm merged PR #18075:
URL: https://github.com/apache/druid/pull/18075
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Dart: Wait for a merge buffer if none is available. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #18075:
URL: https://github.com/apache/druid/pull/18075#discussion_r2143289045
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ProcessingBuffersSet.java:
##########
@@ -59,13 +59,25 @@ public static <T extends Collection<ByteBuffer>> ProcessingBuffersSet fromCollec
);
}
+ /**
+ * Acquire buffers if a particular stages needs them; otherwise, returns null.
+ */
Review Comment:
Fair enough, I made this change.
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ProcessingBuffersProvider.java:
##########
@@ -32,7 +32,7 @@ public interface ProcessingBuffersProvider
/**
* Acquire buffers for a {@link Worker}.
*/
- ResourceHolder<ProcessingBuffersSet> acquire(int poolSize);
+ ResourceHolder<ProcessingBuffersSet> acquire(int poolSize, long timeoutMillis);
Review Comment:
IMO a `long` makes sense because there's only one call site, and it passes `queryDef.getContext().getTimeout()` which is a long.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org