You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2016/09/22 16:12:58 UTC
[2/2] incubator-impala git commit: IMPALA-4037,
IMPALA-4038: fix locking during query cancellation
IMPALA-4037,IMPALA-4038: fix locking during query cancellation
* Refactor the child query handling out of QueryExecState and clarify
locking rules.
* Avoid holding QueryExecState::lock_ while calling
Coordinator::Cancel() or ChildQuery::Cancel(), which can both do RPCs
or acquire ImpalaServer::query_exec_state_map_lock_.
* Fix a potential race between QueryExecState::Exec() and
QueryExecState::Cancel() where the cancelling thread did an unlocked
read of the 'coordinator_' field and may not have cancelled the
coordinator.
Testing:
Ran exhaustive build, ran local stress test for a bit.
Change-Id: Ibe3024803e03595ee69c47759b58e8443d7bd167
Reviewed-on: http://gerrit.cloudera.org:8080/4163
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Internal Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/0d0c93ec
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/0d0c93ec
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/0d0c93ec
Branch: refs/heads/master
Commit: 0d0c93ec8c4949940ec113192731f2adb66a0c5e
Parents: d76a2b2
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Mon Aug 29 11:10:38 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Thu Sep 22 10:07:52 2016 +0000
----------------------------------------------------------------------
be/src/runtime/coordinator.h | 4 +-
be/src/service/child-query.cc | 69 ++++++++++++++++
be/src/service/child-query.h | 66 +++++++++++++++
be/src/service/impala-server.cc | 13 +--
be/src/service/query-exec-state.cc | 125 ++++++++++++++++-------------
be/src/service/query-exec-state.h | 49 +++++------
tests/query_test/test_cancellation.py | 5 --
7 files changed, 227 insertions(+), 104 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0d0c93ec/be/src/runtime/coordinator.h
----------------------------------------------------------------------
diff --git a/be/src/runtime/coordinator.h b/be/src/runtime/coordinator.h
index 0e0b6d5..617ccb9 100644
--- a/be/src/runtime/coordinator.h
+++ b/be/src/runtime/coordinator.h
@@ -246,7 +246,9 @@ class Coordinator {
/// Keeps track of number of completed ranges and total scan ranges.
ProgressUpdater progress_;
- /// protects all fields below
+ /// Protects all fields below. This is held while making RPCs, so this lock should
+ /// only be acquired if the acquiring thread is prepared to wait for a significant
+ /// time.
boost::mutex lock_;
/// Overall status of the entire query; set to the first reported fragment error
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0d0c93ec/be/src/service/child-query.cc
----------------------------------------------------------------------
diff --git a/be/src/service/child-query.cc b/be/src/service/child-query.cc
index 99eb4fe..5b68de4 100644
--- a/be/src/service/child-query.cc
+++ b/be/src/service/child-query.cc
@@ -146,4 +146,73 @@ Status ChildQuery::IsCancelled() {
return Status::CANCELLED;
}
+ChildQueryExecutor::ChildQueryExecutor() : is_cancelled_(false), is_running_(false) {}
+ChildQueryExecutor::~ChildQueryExecutor() {
+ DCHECK(!is_running_);
+}
+
+void ChildQueryExecutor::ExecAsync(vector<ChildQuery>&& child_queries) {
+ DCHECK(!child_queries.empty());
+ lock_guard<SpinLock> lock(lock_);
+ DCHECK(child_queries_.empty());
+ DCHECK(child_queries_thread_.get() == NULL);
+ if (is_cancelled_) return;
+ child_queries_ = move(child_queries);
+ child_queries_thread_.reset(new Thread("query-exec-state", "async child queries",
+ bind(&ChildQueryExecutor::ExecChildQueries, this)));
+ is_running_ = true;
+}
+
+void ChildQueryExecutor::ExecChildQueries() {
+ for (ChildQuery& child_query : child_queries_) {
+ // Execute without holding 'lock_'.
+ Status status = child_query.ExecAndFetch();
+ if (!status.ok()) {
+ lock_guard<SpinLock> lock(lock_);
+ child_queries_status_ = status;
+ break;
+ }
+ }
+
+ {
+ lock_guard<SpinLock> lock(lock_);
+ is_running_ = false;
+ }
+}
+
+Status ChildQueryExecutor::WaitForAll(vector<ChildQuery*>* completed_queries) {
+ // Safe to read without lock since we don't call this concurrently with ExecAsync().
+ if (child_queries_thread_ == NULL) {
+ DCHECK(!is_running_);
+ return Status::OK();
+ }
+ child_queries_thread_->Join();
+
+ // Safe to read below fields without 'lock_' because they are immutable after the
+ // thread finishes.
+ RETURN_IF_ERROR(child_queries_status_);
+ for (ChildQuery& child_query : child_queries_) {
+ completed_queries->push_back(&child_query);
+ }
+ return Status::OK();
+}
+
+void ChildQueryExecutor::Cancel() {
+ {
+ lock_guard<SpinLock> l(lock_);
+ // Prevent more child queries from starting. After this critical section,
+ // 'child_queries_' will not be modified.
+ is_cancelled_ = true;
+ if (!is_running_) return;
+ DCHECK_EQ(child_queries_thread_ == NULL, child_queries_.empty());
+ }
+
+ // Cancel child queries without holding 'lock_'.
+ // Safe because 'child_queries_' and 'child_queries_thread_' are immutable after
+ // cancellation.
+ for (ChildQuery& child_query : child_queries_) {
+ child_query.Cancel();
+ }
+ child_queries_thread_->Join();
+}
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0d0c93ec/be/src/service/child-query.h
----------------------------------------------------------------------
diff --git a/be/src/service/child-query.h b/be/src/service/child-query.h
index 2fdcc5a..1c7a20e 100644
--- a/be/src/service/child-query.h
+++ b/be/src/service/child-query.h
@@ -142,6 +142,72 @@ class ChildQuery {
bool is_cancelled_;
};
+/// Asynchronously executes a set of child queries in a separate thread.
+///
+/// ExecAsync() is called at most once per executor to execute a set of child queries
+/// asynchronously. After ExecAsync() is called, either WaitForAll() or Cancel() must be
+/// called to ensure that the child queries are no longer executing before destroying the
+/// object.
+class ChildQueryExecutor {
+ public:
+ ChildQueryExecutor();
+ ~ChildQueryExecutor();
+
+ /// Asynchronously executes 'child_queries' one by one in a new thread. 'child_queries'
+ /// must be non-empty. May clear or modify the 'child_queries' arg. Can only be called
+ /// once. Does nothing if Cancel() was already called.
+ void ExecAsync(std::vector<ChildQuery>&& child_queries);
+
+ /// Waits for all child queries to complete successfully or with an error. Returns a
+ /// non-OK status if a child query fails. Returns OK if ExecAsync() was not called,
+ /// Cancel() was called before an error occurred, or if all child queries finished
+ /// successfully. If returning OK, populates 'completed_queries' with the completed
+ /// queries. Any returned ChildQueries remain owned by the executor. Should not be
+ /// called concurrently with ExecAsync(). After WaitForAll() returns, the object can
+ /// safely be destroyed.
+ Status WaitForAll(std::vector<ChildQuery*>* completed_queries);
+
+ /// Cancels all child queries and prevents any more from starting. Returns once all
+ /// child queries are cancelled, after which the object can safely be destroyed. Can
+ /// be safely called concurrently with ExecAsync() or WaitForAll().
+ void Cancel();
+
+ private:
+ /// Serially executes the queries in child_queries_ by calling the child query's
+ /// ExecAndWait(). This function blocks until all queries complete and is run
+ /// in 'child_queries_thread_'.
+ /// Sets 'child_queries_status_'.
+ void ExecChildQueries();
+
+ /// Protects all fields below.
+ /// Should not be held at the same time as 'ChildQuery::lock_'.
+ SpinLock lock_;
+
+ /// True if cancellation of child queries has been initiated and no more child queries
+ /// should be started.
+ bool is_cancelled_;
+
+ /// True if 'child_queries_thread_' is in the process of executing child queries.
+ /// Set to false by 'child_queries_thread_' just before it exits. 'is_running_' must
+ /// be false when ChildQueryExecutor is destroyed: once execution is started,
+ /// WaitForAll() or Cancel() must be called to ensure the thread exits.
+ bool is_running_;
+
+ /// List of child queries to be executed. Not modified after it is initially populated,
+ /// so safe to read without holding 'lock_' if 'is_running_' or 'is_cancelled_' is
+ /// true, or 'child_queries_thread_' is non-NULL.
+ std::vector<ChildQuery> child_queries_;
+
+ /// Thread to execute 'child_queries_' in. Immutable after the first time it is set or
+ /// after 'is_cancelled_' is true.
+ boost::scoped_ptr<Thread> child_queries_thread_;
+
+ /// The status of the child queries. The status is OK iff all child queries complete
+ /// successfully. Otherwise, status contains the error of the first child query that
+ /// failed (child queries are executed serially and abort on the first error).
+ /// Immutable after 'child_queries_thread_' exits
+ Status child_queries_status_;
+};
}
#endif
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0d0c93ec/be/src/service/impala-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index 1b10aec..d5fd59a 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -995,18 +995,9 @@ Status ImpalaServer::UpdateCatalogMetrics() {
Status ImpalaServer::CancelInternal(const TUniqueId& query_id, bool check_inflight,
const Status* cause) {
VLOG_QUERY << "Cancel(): query_id=" << PrintId(query_id);
- shared_ptr<QueryExecState> exec_state = GetQueryExecState(query_id, true);
+ shared_ptr<QueryExecState> exec_state = GetQueryExecState(query_id, false);
if (exec_state == NULL) return Status("Invalid or unknown query handle");
- lock_guard<mutex> l(*exec_state->lock(), adopt_lock_t());
- if (check_inflight) {
- lock_guard<mutex> l2(exec_state->session()->lock);
- if (exec_state->session()->inflight_queries.find(query_id) ==
- exec_state->session()->inflight_queries.end()) {
- return Status("Query not yet running");
- }
- }
- // TODO: can we call Coordinator::Cancel() here while holding lock?
- exec_state->Cancel(cause);
+ exec_state->Cancel(check_inflight, cause);
return Status::OK();
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0d0c93ec/be/src/service/query-exec-state.cc
----------------------------------------------------------------------
diff --git a/be/src/service/query-exec-state.cc b/be/src/service/query-exec-state.cc
index eb710fb..9b2fc88 100644
--- a/be/src/service/query-exec-state.cc
+++ b/be/src/service/query-exec-state.cc
@@ -64,6 +64,7 @@ ImpalaServer::QueryExecState::QueryExecState(
: query_ctx_(query_ctx),
last_active_time_(numeric_limits<int64_t>::max()),
ref_count_(0L),
+ child_query_executor_(new ChildQueryExecutor),
exec_env_(exec_env),
is_block_on_wait_joining_(false),
session_(session),
@@ -73,6 +74,7 @@ ImpalaServer::QueryExecState::QueryExecState(
profile_(&profile_pool_, "Query"), // assign name w/ id after planning
server_profile_(&profile_pool_, "ImpalaServer"),
summary_profile_(&profile_pool_, "Summary"),
+ is_cancelled_(false),
eos_(false),
query_state_(beeswax::QueryState::CREATED),
current_batch_(NULL),
@@ -426,9 +428,14 @@ Status ImpalaServer::QueryExecState::ExecQueryOrDmlRequest(
query_exec_request.fragments[0].partition.type == TPartitionType::UNPARTITIONED;
DCHECK(has_coordinator_fragment || query_exec_request.__isset.desc_tbl);
- schedule_.reset(new QuerySchedule(query_id(), query_exec_request,
- exec_request_.query_options, &summary_profile_, query_events_));
- coord_.reset(new Coordinator(exec_request_.query_options, exec_env_, query_events_));
+ {
+ lock_guard<mutex> l(lock_);
+ // Don't start executing the query if Cancel() was called concurrently with Exec().
+ if (is_cancelled_) return Status::CANCELLED;
+ schedule_.reset(new QuerySchedule(query_id(), query_exec_request,
+ exec_request_.query_options, &summary_profile_, query_events_));
+ coord_.reset(new Coordinator(exec_request_.query_options, exec_env_, query_events_));
+ }
Status status = exec_env_->scheduler()->Schedule(coord_.get(), schedule_.get());
{
@@ -462,15 +469,17 @@ Status ImpalaServer::QueryExecState::ExecDdlRequest() {
TComputeStatsParams& compute_stats_params =
exec_request_.catalog_op_request.ddl_params.compute_stats_params;
// Add child queries for computing table and column stats.
+ vector<ChildQuery> child_queries;
if (compute_stats_params.__isset.tbl_stats_query) {
- child_queries_.push_back(
+ child_queries.push_back(
ChildQuery(compute_stats_params.tbl_stats_query, this, parent_server_));
}
if (compute_stats_params.__isset.col_stats_query) {
- child_queries_.push_back(
+ child_queries.push_back(
ChildQuery(compute_stats_params.col_stats_query, this, parent_server_));
}
- if (child_queries_.size() > 0) ExecChildQueriesAsync();
+
+ if (child_queries.size() > 0) child_query_executor_->ExecAsync(move(child_queries));
return Status::OK();
}
@@ -600,7 +609,15 @@ Status ImpalaServer::QueryExecState::WaitInternal() {
return Status::OK();
}
- RETURN_IF_ERROR(WaitForChildQueries());
+ vector<ChildQuery*> child_queries;
+ Status child_queries_status = child_query_executor_->WaitForAll(&child_queries);
+ {
+ lock_guard<mutex> l(lock_);
+ RETURN_IF_ERROR(query_status_);
+ RETURN_IF_ERROR(UpdateQueryStatus(child_queries_status));
+ }
+ query_events_->MarkEvent("Child queries finished");
+
if (coord_.get() != NULL) {
RETURN_IF_ERROR(coord_->Wait());
RETURN_IF_ERROR(Expr::Open(output_expr_ctxs_, coord_->runtime_state()));
@@ -608,8 +625,8 @@ Status ImpalaServer::QueryExecState::WaitInternal() {
}
if (catalog_op_type() == TCatalogOpType::DDL &&
- ddl_type() == TDdlType::COMPUTE_STATS && child_queries_.size() > 0) {
- RETURN_IF_ERROR(UpdateTableAndColumnStats());
+ ddl_type() == TDdlType::COMPUTE_STATS && child_queries.size() > 0) {
+ RETURN_IF_ERROR(UpdateTableAndColumnStats(child_queries));
}
if (!returns_result_set()) {
@@ -820,22 +837,40 @@ Status ImpalaServer::QueryExecState::GetRowValue(TupleRow* row, vector<void*>* r
return Status::OK();
}
-void ImpalaServer::QueryExecState::Cancel(const Status* cause) {
- // Cancel and close child queries before cancelling parent.
- for (ChildQuery& child_query: child_queries_) {
- child_query.Cancel();
- }
-
- // If the query is completed or cancelled, no need to cancel.
- if (eos_ || query_state_ == QueryState::EXCEPTION) return;
+Status ImpalaServer::QueryExecState::Cancel(bool check_inflight, const Status* cause) {
+ Coordinator* coord;
+ {
+ lock_guard<mutex> lock(lock_);
+ if (check_inflight) {
+ lock_guard<mutex> session_lock(session_->lock);
+ if (session_->inflight_queries.find(query_id()) ==
+ session_->inflight_queries.end()) {
+ return Status("Query not yet running");
+ }
+ }
- if (cause != NULL) {
- DCHECK(!cause->ok());
- UpdateQueryStatus(*cause);
- query_events_->MarkEvent("Cancelled");
- DCHECK_EQ(query_state_, QueryState::EXCEPTION);
- }
- if (coord_.get() != NULL) coord_->Cancel(cause);
+ // If the query is completed or cancelled, no need to update state.
+ bool already_done = eos_ || query_state_ == QueryState::EXCEPTION;
+ if (!already_done && cause != NULL) {
+ DCHECK(!cause->ok());
+ UpdateQueryStatus(*cause);
+ query_events_->MarkEvent("Cancelled");
+ DCHECK_EQ(query_state_, QueryState::EXCEPTION);
+ }
+ // Get a copy of the coordinator pointer while holding 'lock_'.
+ coord = coord_.get();
+ is_cancelled_ = true;
+ } // Release lock_ before doing cancellation work.
+
+ // Cancel and close child queries before cancelling parent. 'lock_' should not be held
+ // because a) ChildQuery::Cancel() calls back into ImpalaServer and b) cancellation
+ // involves RPCs and can take quite some time.
+ child_query_executor_->Cancel();
+
+ // Cancel the parent query. 'lock_' should not be held because cancellation involves
+ // RPCs and can block for a long time.
+ if (coord != NULL) coord->Cancel(cause);
+ return Status::OK();
}
Status ImpalaServer::QueryExecState::UpdateCatalog() {
@@ -988,9 +1023,10 @@ void ImpalaServer::QueryExecState::MarkActive() {
++ref_count_;
}
-Status ImpalaServer::QueryExecState::UpdateTableAndColumnStats() {
- DCHECK_GE(child_queries_.size(), 1);
- DCHECK_LE(child_queries_.size(), 2);
+Status ImpalaServer::QueryExecState::UpdateTableAndColumnStats(
+ const vector<ChildQuery*>& child_queries) {
+ DCHECK_GE(child_queries.size(), 1);
+ DCHECK_LE(child_queries.size(), 2);
catalog_op_executor_.reset(
new CatalogOpExecutor(exec_env_, frontend_, &server_profile_));
@@ -998,15 +1034,15 @@ Status ImpalaServer::QueryExecState::UpdateTableAndColumnStats() {
// ExecComputeStats(). Otherwise pass in the column stats result.
TTableSchema col_stats_schema;
TRowSet col_stats_data;
- if (child_queries_.size() > 1) {
- col_stats_schema = child_queries_[1].result_schema();
- col_stats_data = child_queries_[1].result_data();
+ if (child_queries.size() > 1) {
+ col_stats_schema = child_queries[1]->result_schema();
+ col_stats_data = child_queries[1]->result_data();
}
Status status = catalog_op_executor_->ExecComputeStats(
exec_request_.catalog_op_request.ddl_params.compute_stats_params,
- child_queries_[0].result_schema(),
- child_queries_[0].result_data(),
+ child_queries[0]->result_schema(),
+ child_queries[0]->result_data(),
col_stats_schema,
col_stats_data);
{
@@ -1030,31 +1066,6 @@ Status ImpalaServer::QueryExecState::UpdateTableAndColumnStats() {
return Status::OK();
}
-void ImpalaServer::QueryExecState::ExecChildQueriesAsync() {
- DCHECK(child_queries_thread_.get() == NULL);
- child_queries_thread_.reset(new Thread("query-exec-state", "async child queries",
- bind(&ImpalaServer::QueryExecState::ExecChildQueries, this)));
-}
-
-void ImpalaServer::QueryExecState::ExecChildQueries() {
- for (int i = 0; i < child_queries_.size(); ++i) {
- if (!child_queries_status_.ok()) return;
- child_queries_status_ = child_queries_[i].ExecAndFetch();
- }
-}
-
-Status ImpalaServer::QueryExecState::WaitForChildQueries() {
- if (child_queries_thread_.get() == NULL) return Status::OK();
- child_queries_thread_->Join();
- {
- lock_guard<mutex> l(lock_);
- RETURN_IF_ERROR(query_status_);
- RETURN_IF_ERROR(UpdateQueryStatus(child_queries_status_));
- }
- query_events_->MarkEvent("Child queries finished");
- return Status::OK();
-}
-
void ImpalaServer::QueryExecState::ClearResultCache() {
if (result_cache_ == NULL) return;
// Update result set cache metrics and mem limit accounting.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0d0c93ec/be/src/service/query-exec-state.h
----------------------------------------------------------------------
diff --git a/be/src/service/query-exec-state.h b/be/src/service/query-exec-state.h
index feb8afe..0a763ff 100644
--- a/be/src/service/query-exec-state.h
+++ b/be/src/service/query-exec-state.h
@@ -124,9 +124,12 @@ class ImpalaServer::QueryExecState {
/// Cancels the child queries and the coordinator with the given cause.
/// If cause is NULL, assume this was deliberately cancelled by the user.
/// Otherwise, sets state to EXCEPTION.
- /// Caller needs to hold lock_.
/// Does nothing if the query has reached EOS or already cancelled.
- void Cancel(const Status* cause = NULL);
+ ///
+ /// Only returns an error if 'check_inflight' is true and the query is not yet
+ /// in-flight. Otherwise, proceed and return Status::OK() even if the query isn't
+ /// in-flight (for cleaning up after an error on the query issuing path).
+ Status Cancel(bool check_inflight, const Status* cause);
/// This is called when the query is done (finished, cancelled, or failed).
/// Takes lock_: callers must not hold lock() before calling.
@@ -217,7 +220,16 @@ class ImpalaServer::QueryExecState {
/// increased, and decreased once that work is completed.
uint32_t ref_count_;
- boost::mutex lock_; // protects all following fields
+ /// Executor for any child queries (e.g. compute stats subqueries). Always non-NULL.
+ const boost::scoped_ptr<ChildQueryExecutor> child_query_executor_;
+
+ // Protects all following fields. Acquirers should be careful not to hold it for too
+ // long, e.g. during RPCs because this lock is required to make progress on various
+ // ImpalaServer requests. If held for too long it can block progress of client
+ // requests for this query, e.g. query status and cancellation. Furthermore, until
+ // IMPALA-3882 is fixed, it can indirectly block progress on all other queries.
+ boost::mutex lock_;
+
ExecEnv* exec_env_;
/// Thread for asynchronously running Wait().
@@ -282,6 +294,7 @@ class ImpalaServer::QueryExecState {
RuntimeProfile::EventSequence* query_events_;
std::vector<ExprContext*> output_expr_ctxs_;
+ bool is_cancelled_; // if true, Cancel() was called.
bool eos_; // if true, there are no more rows to return
// We enforce the invariant that query_status_ is not OK iff query_state_
// is EXCEPTION, given that lock_ is held.
@@ -308,16 +321,6 @@ class ImpalaServer::QueryExecState {
/// Start/end time of the query
TimestampValue start_time_, end_time_;
- /// List of child queries to be executed on behalf of this query.
- std::vector<ChildQuery> child_queries_;
-
- /// Thread to execute child_queries_ in and the resulting status. The status is OK iff
- /// all child queries complete successfully. Otherwise, status contains the error of the
- /// first child query that failed (child queries are executed serially and abort on the
- /// first error).
- Status child_queries_status_;
- boost::scoped_ptr<Thread> child_queries_thread_;
-
/// Executes a local catalog operation (an operation that does not need to execute
/// against the catalog service). Includes USE, SHOW, DESCRIBE, and EXPLAIN statements.
Status ExecLocalCatalogOp(const TCatalogOpRequest& catalog_op);
@@ -333,6 +336,8 @@ class ImpalaServer::QueryExecState {
/// Core logic of initiating a query or dml execution request.
/// Initiates execution of plan fragments, if there are any, and sets
/// up the output exprs for subsequent calls to FetchRows().
+ /// 'coord_' is only valid after this method is called, and may be invalid if it
+ /// returns an error.
/// Also sets up profile and pre-execution counters.
/// Non-blocking.
Status ExecQueryOrDmlRequest(const TQueryExecRequest& query_exec_request);
@@ -386,23 +391,7 @@ class ImpalaServer::QueryExecState {
/// For example, INSERT queries update partition metadata in UpdateCatalog() using a
/// TUpdateCatalogRequest, whereas our DDL uses a TCatalogOpRequest for very similar
/// purposes. Perhaps INSERT should use a TCatalogOpRequest as well.
- Status UpdateTableAndColumnStats();
-
- /// Asynchronously executes all child_queries_ one by one. Calls ExecChildQueries()
- /// in a new child_queries_thread_.
- void ExecChildQueriesAsync();
-
- /// Serially executes the queries in child_queries_ by calling the child query's
- /// ExecAndWait(). This function is blocking and is intended to be run in a separate
- /// thread to ensure that Exec() remains non-blocking. Sets child_queries_status_.
- /// Must not be called while holding lock_.
- void ExecChildQueries();
-
- /// Waits for all child queries to complete successfully or with an error, by joining
- /// child_queries_thread_. Returns a non-OK status if a child query fails or if the
- /// parent query is cancelled (subsequent children will not be executed). Returns OK
- /// if child_queries_thread_ is not set or if all child queries finished successfully.
- Status WaitForChildQueries();
+ Status UpdateTableAndColumnStats(const std::vector<ChildQuery*>& child_queries);
/// Sets result_cache_ to NULL and updates its associated metrics and mem consumption.
/// This function is a no-op if the cache has already been cleared.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0d0c93ec/tests/query_test/test_cancellation.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_cancellation.py b/tests/query_test/test_cancellation.py
index c7cc6da..265c781 100644
--- a/tests/query_test/test_cancellation.py
+++ b/tests/query_test/test_cancellation.py
@@ -72,11 +72,6 @@ class TestCancellation(ImpalaTestSuite):
# Ignore 'compute stats' queries for the CTAS query type.
cls.TestMatrix.add_constraint(lambda v: not (v.get_value('query_type') == 'CTAS' and
v.get_value('query').startswith('compute stats')))
- # Ignore debug actions for 'compute stats' because cancellation of 'compute stats'
- # relies on child queries eventually making forward progress, but debug actions
- # will cause child queries to hang indefinitely.
- cls.TestMatrix.add_constraint(lambda v: not (v.get_value('action') == 'WAIT' and
- v.get_value('query').startswith('compute stats')))
# tpch tables are not generated for hbase as the data loading takes a very long time.
# TODO: Add cancellation tests for hbase.
cls.TestMatrix.add_constraint(lambda v:\