incubator-impala git commit: IMPALA-4905: Don't send empty insert status to coordinator

[he...@apache.org]

Repository: incubator-impala
Updated Branches:
  refs/heads/master 7a6008b9d -> d25db64f0


IMPALA-4905: Don't send empty insert status to coordinator

If the coordinator, in UpdateBackendExecStatus(), receives a report that
includes a TInsertExecStatus, it will call UpdateInsertExecStatus()
which takes the coordinator-wide lock. Avoid doing this for fragment
instances that would only send an empty TInsertExecStatus (including
instances that belong to SELECT queries, not DML queries).

Future changes should fix the locking around the
UpdateBackendExecStatus() path to remove dependencies on
Coordinator::lock_, but this fix is simple and addresses one point of
needless contention.

Change-Id: I314dd8d96922d273c6329266970d249ec8c5c608
Reviewed-on: http://gerrit.cloudera.org:8080/7457
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Impala Public 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/d25db64f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/d25db64f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/d25db64f

Branch: refs/heads/master
Commit: d25db64f0e17092af9ef60eb37ec9214900c2d1c
Parents: 7a6008b
Author: Henry Robinson <he...@cloudera.com>
Authored: Tue Jul 18 15:20:42 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Jul 25 00:09:19 2017 +0000

----------------------------------------------------------------------
 be/src/runtime/query-state.cc | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d25db64f/be/src/runtime/query-state.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/query-state.cc b/be/src/runtime/query-state.cc
index 366b79f..21f35fb 100644
--- a/be/src/runtime/query-state.cc
+++ b/be/src/runtime/query-state.cc
@@ -201,22 +201,23 @@ void QueryState::ReportExecStatusAux(bool done, const Status& status,
       instance_status.__isset.profile = true;
     }
 
-    // Only send updates to insert status if fragment is finished, the coordinator
-    // waits until query execution is done to use them anyhow.
-    if (done) {
+    // Only send updates to insert status if fragment is finished, the coordinator waits
+    // until query execution is done to use them anyhow.
+    RuntimeState* state = fis->runtime_state();
+    if (done && (state->hdfs_files_to_move()->size() > 0
+        || state->per_partition_status()->size() > 0)) {
       TInsertExecStatus insert_status;
-      if (fis->runtime_state()->hdfs_files_to_move()->size() > 0) {
-        insert_status.__set_files_to_move(*fis->runtime_state()->hdfs_files_to_move());
+      if (state->hdfs_files_to_move()->size() > 0) {
+        insert_status.__set_files_to_move(*state->hdfs_files_to_move());
       }
-      if (fis->runtime_state()->per_partition_status()->size() > 0) {
-        insert_status.__set_per_partition_status(
-            *fis->runtime_state()->per_partition_status());
+      if (state->per_partition_status()->size() > 0) {
+        insert_status.__set_per_partition_status(*state->per_partition_status());
       }
       params.__set_insert_exec_status(insert_status);
     }
 
     // Send new errors to coordinator
-    fis->runtime_state()->GetUnreportedErrors(&params.error_log);
+    state->GetUnreportedErrors(&params.error_log);
     params.__isset.error_log = (params.error_log.size() > 0);
   }