You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "zhztheplayer (via GitHub)" <gi...@apache.org> on 2026/01/30 09:36:10 UTC
[PR] [GLUTEN-11514][VL] Add `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
zhztheplayer opened a new pull request, #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527
New APIs adding to `ColumnarBatchOutIterator`:
- `ColumnarBatchOutIterator#addIteratorSplits`
- `ColumnarBatchOutIterator#noMoreSplits`
This allows iterator-based splits to be added later than when the task is started. Useful when `NativePlanEvaluator` is used individually, out of the transformer code routine.
This is a prerequisite for https://github.com/apache/incubator-gluten/issues/11514 and https://github.com/apache/incubator-gluten/pull/11419.
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Refactor plan execution by adding `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#discussion_r2759571890
##########
cpp/velox/compute/WholeStageResultIterator.cc:
##########
@@ -355,17 +357,39 @@ void WholeStageResultIterator::constructPartitionColumns(
}
}
-void WholeStageResultIterator::tryAddSplitsToTask() {
- if (noMoreSplits_) {
+void WholeStageResultIterator::addIteratorSplits(const std::vector<std::shared_ptr<ResultIterator>>& inputIterators) {
+ // Create IteratorConnectorSplit for each iterator
+ for (size_t i = 0; i < streamIds_.size() && i < inputIterators.size(); ++i) {
+ if (inputIterators[i] == nullptr) {
+ continue;
+ }
+ auto connectorSplit = std::make_shared<IteratorConnectorSplit>(
+ kIteratorConnectorId, inputIterators[i]);
+ exec::Split split(folly::copy(connectorSplit), -1);
+ task_->addSplit(streamIds_[i], std::move(split));
+ }
+}
+
+void WholeStageResultIterator::noMoreSplits() {
Review Comment:
It's not guaranteed thread-safe. In the Delta PR, the plan is still created and executed in one single thread so no requirement for thread safety on this API now.
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Add `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#discussion_r2758078917
##########
cpp/velox/operators/plannodes/CudfVectorStream.h:
##########
@@ -18,21 +18,90 @@
#pragma once
#include "CudfVectorStream.h"
+#include "compute/ResultIterator.h"
+#include "memory/VeloxColumnarBatch.h"
+#include "velox/exec/Driver.h"
+#include "velox/exec/Operator.h"
+#include "velox/exec/Task.h"
#include "velox/experimental/cudf/exec/CudfOperator.h"
#include "velox/experimental/cudf/exec/Utilities.h"
#include "velox/experimental/cudf/exec/VeloxCudfInterop.h"
#include "velox/experimental/cudf/vector/CudfVector.h"
namespace gluten {
-class CudfVectorStream : public RowVectorStream {
+class CudfVectorStreamBase {
Review Comment:
cc @jinchengchenghh
`RowVectorStream`'s code is being inlined into `CudfVectorStreamBase` here.
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Add `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#issuecomment-3824056015
Run Gluten Clickhouse CI on x86
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Refactor plan execution by adding `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan commented on code in PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#discussion_r2759146556
##########
cpp/velox/compute/WholeStageResultIterator.cc:
##########
@@ -355,17 +357,39 @@ void WholeStageResultIterator::constructPartitionColumns(
}
}
-void WholeStageResultIterator::tryAddSplitsToTask() {
- if (noMoreSplits_) {
+void WholeStageResultIterator::addIteratorSplits(const std::vector<std::shared_ptr<ResultIterator>>& inputIterators) {
+ // Create IteratorConnectorSplit for each iterator
Review Comment:
should we also guard for `allSplitsAdded = true`?
##########
cpp/velox/compute/WholeStageResultIterator.cc:
##########
@@ -355,17 +357,39 @@ void WholeStageResultIterator::constructPartitionColumns(
}
}
-void WholeStageResultIterator::tryAddSplitsToTask() {
- if (noMoreSplits_) {
+void WholeStageResultIterator::addIteratorSplits(const std::vector<std::shared_ptr<ResultIterator>>& inputIterators) {
+ // Create IteratorConnectorSplit for each iterator
+ for (size_t i = 0; i < streamIds_.size() && i < inputIterators.size(); ++i) {
+ if (inputIterators[i] == nullptr) {
+ continue;
+ }
+ auto connectorSplit = std::make_shared<IteratorConnectorSplit>(
+ kIteratorConnectorId, inputIterators[i]);
+ exec::Split split(folly::copy(connectorSplit), -1);
+ task_->addSplit(streamIds_[i], std::move(split));
+ }
+}
+
+void WholeStageResultIterator::noMoreSplits() {
Review Comment:
Would this work if calling from different threads? e.g. thread A called addIteratorSplits(), but thread B called on noMoreSplits()
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Add `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#issuecomment-3824626506
Run Gluten Clickhouse CI on x86
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Add `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh commented on code in PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#discussion_r2758231672
##########
cpp/velox/operators/plannodes/CudfVectorStream.h:
##########
@@ -18,21 +18,90 @@
#pragma once
#include "CudfVectorStream.h"
+#include "compute/ResultIterator.h"
+#include "memory/VeloxColumnarBatch.h"
+#include "velox/exec/Driver.h"
+#include "velox/exec/Operator.h"
+#include "velox/exec/Task.h"
#include "velox/experimental/cudf/exec/CudfOperator.h"
#include "velox/experimental/cudf/exec/Utilities.h"
#include "velox/experimental/cudf/exec/VeloxCudfInterop.h"
#include "velox/experimental/cudf/vector/CudfVector.h"
namespace gluten {
-class CudfVectorStream : public RowVectorStream {
+class CudfVectorStreamBase {
Review Comment:
ok
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Refactor plan execution by adding `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer merged PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Add `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#issuecomment-3825490551
Run Gluten Clickhouse CI on x86
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Add `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#issuecomment-3840264916
Run Gluten Clickhouse CI on x86
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Refactor plan execution by adding `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#discussion_r2759571890
##########
cpp/velox/compute/WholeStageResultIterator.cc:
##########
@@ -355,17 +357,39 @@ void WholeStageResultIterator::constructPartitionColumns(
}
}
-void WholeStageResultIterator::tryAddSplitsToTask() {
- if (noMoreSplits_) {
+void WholeStageResultIterator::addIteratorSplits(const std::vector<std::shared_ptr<ResultIterator>>& inputIterators) {
+ // Create IteratorConnectorSplit for each iterator
+ for (size_t i = 0; i < streamIds_.size() && i < inputIterators.size(); ++i) {
+ if (inputIterators[i] == nullptr) {
+ continue;
+ }
+ auto connectorSplit = std::make_shared<IteratorConnectorSplit>(
+ kIteratorConnectorId, inputIterators[i]);
+ exec::Split split(folly::copy(connectorSplit), -1);
+ task_->addSplit(streamIds_[i], std::move(split));
+ }
+}
+
+void WholeStageResultIterator::noMoreSplits() {
Review Comment:
It's not guaranteed thread-safe. In the Delta PR, the plan is still created and executed in one single thread.
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Add `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#issuecomment-3823982500
Run Gluten Clickhouse CI on x86
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Refactor plan execution by adding `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#issuecomment-3841936103
Run Gluten Clickhouse CI on x86
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Refactor plan execution by adding `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#issuecomment-3842063599
@zhouyuan
> Thanks! I understand the basic logic now - just one question on the concurrent execution threading model
The change is not supposed to support a concurrent execution model - it's still single-threaded execution. The goal was to add the barrier API, so we can reuse the Velox task in the same thread but for newer inputs.
Let me know any remaining issues. cc @rui-mo
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Add `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#issuecomment-3840293085
Run Gluten Clickhouse CI on x86
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Add `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#issuecomment-3825141164
Run Gluten Clickhouse CI on x86
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Refactor plan execution by adding `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#discussion_r2759571890
##########
cpp/velox/compute/WholeStageResultIterator.cc:
##########
@@ -355,17 +357,39 @@ void WholeStageResultIterator::constructPartitionColumns(
}
}
-void WholeStageResultIterator::tryAddSplitsToTask() {
- if (noMoreSplits_) {
+void WholeStageResultIterator::addIteratorSplits(const std::vector<std::shared_ptr<ResultIterator>>& inputIterators) {
+ // Create IteratorConnectorSplit for each iterator
+ for (size_t i = 0; i < streamIds_.size() && i < inputIterators.size(); ++i) {
+ if (inputIterators[i] == nullptr) {
+ continue;
+ }
+ auto connectorSplit = std::make_shared<IteratorConnectorSplit>(
+ kIteratorConnectorId, inputIterators[i]);
+ exec::Split split(folly::copy(connectorSplit), -1);
+ task_->addSplit(streamIds_[i], std::move(split));
+ }
+}
+
+void WholeStageResultIterator::noMoreSplits() {
Review Comment:
I don't expect this usage as of now. In the Delta PR, the plan is still created and executed in one single thread.
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11514][VL] Refactor plan execution by adding `addIteratorSplits` and `noMoreSplits` methods to the plan execution API [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #11527:
URL: https://github.com/apache/incubator-gluten/pull/11527#discussion_r2759577627
##########
cpp/velox/compute/WholeStageResultIterator.cc:
##########
@@ -355,17 +357,39 @@ void WholeStageResultIterator::constructPartitionColumns(
}
}
-void WholeStageResultIterator::tryAddSplitsToTask() {
- if (noMoreSplits_) {
+void WholeStageResultIterator::addIteratorSplits(const std::vector<std::shared_ptr<ResultIterator>>& inputIterators) {
+ // Create IteratorConnectorSplit for each iterator
Review Comment:
Yes, good idea.
--
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@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org