Skip to content

Commit

Permalink
De-flake DriverTest.pause (#11202)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #11202

This diff fixes a rare race between test shutdown and Task shutdowm. Even though Test shutdown waits for Task constructor to finish, Test shutdown was not waiting for all member variables of Task to be cleaned up: for example task.queryCtx_ holds memory pool and if task.queryCtx_ is not freed before test shutdown, we get test failure. This diff fixes queryCtx_ destruction by reseting the pointer in ~Task.

Reviewed By: xiaoxmeng

Differential Revision: D64026457

fbshipit-source-id: f0c350530b7f1c8cbfc051df3e12ad4d0b95e364
  • Loading branch information
pansatadru authored and facebook-github-bot committed Oct 9, 2024
1 parent 6a7c844 commit acd5717
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
5 changes: 3 additions & 2 deletions velox/exec/Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,11 @@ Task::Task(
std::function<void(std::exception_ptr)> onError)
: uuid_{makeUuid()},
taskId_(taskId),
planFragment_(std::move(planFragment)),
destination_(destination),
mode_(mode),
queryCtx_(std::move(queryCtx)),
planFragment_(std::move(planFragment)),
traceConfig_(maybeMakeTraceConfig()),
mode_(mode),
consumerSupplier_(std::move(consumerSupplier)),
onError_(std::move(onError)),
splitsStates_(buildSplitStates(planFragment_.planNode)),
Expand Down Expand Up @@ -347,6 +347,7 @@ Task::~Task() {
CLEAR(childPools_.clear());
CLEAR(pool_.reset());
CLEAR(planFragment_ = core::PlanFragment());
CLEAR(queryCtx_.reset());
clearStage = "exiting ~Task()";

// Ful-fill the task deletion promises at the end.
Expand Down
10 changes: 7 additions & 3 deletions velox/exec/Task.h
Original file line number Diff line number Diff line change
Expand Up @@ -992,15 +992,19 @@ class Task : public std::enable_shared_from_this<Task> {
// Application specific task ID specified at construction time. May not be
// unique or universally unique.
const std::string taskId_;
core::PlanFragment planFragment_;

const int destination_;
const std::shared_ptr<core::QueryCtx> queryCtx_;
const std::optional<trace::QueryTraceConfig> traceConfig_;

// The execution mode of the task. It is enforced that a task can only be
// executed in a single mode throughout its lifetime
const ExecutionMode mode_;

std::shared_ptr<core::QueryCtx> queryCtx_;

core::PlanFragment planFragment_;

const std::optional<trace::QueryTraceConfig> traceConfig_;

// Hook in the system wide task list.
TaskListEntry taskListEntry_;

Expand Down

0 comments on commit acd5717

Please sign in to comment.