westonpace commented on code in PR #13651:
URL: https://github.com/apache/arrow/pull/13651#discussion_r925696912
##########
cpp/src/arrow/compute/exec/exec_plan.h:
##########
@@ -69,17 +69,16 @@ class ARROW_EXPORT ExecPlan : public
std::enable_shared_from_this<ExecPlan> {
/// e.g. make an array of thread-locals off this.
size_t max_concurrency() const;
- /// \brief Add a future to the plan's task group.
+ /// \brief Starts an external task
///
- /// \param fut The future to add
+ /// This should be avoided if possible. It is kept in for now for legacy
+ /// purposes. This should be called before the external task is started. If
+ /// a future is returned then it should be marked complete when the external
+ /// task has finished.
///
- /// Use this when interfacing with anything that returns a future (such as
IO), but
- /// prefer ScheduleTask/StartTaskGroup inside of ExecNodes.
- /// The below API interfaces with the scheduler to add tasks to the task
group. Tasks
- /// should be added sparingly! Prefer just doing the work immediately rather
than adding
- /// a task for it. Tasks are used in pipeline breakers that may output many
more rows
- /// than they received (such as a full outer join).
- Status AddFuture(Future<> fut);
+ /// \return nullopt if the plan has already ended, otherwise this returns
+ /// a future that must be completed when the external task finishes
+ Result<util::optional<Future<>>> BeginExternalTask();
Review Comment:
Good idea. I removed `optional`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
