You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2019/11/20 21:59:54 UTC

[impala] branch master updated (9d8b846 -> fc4a91c)

This is an automated email from the ASF dual-hosted git repository.

joemcdonnell pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.


    from 9d8b846  IMPALA-9171: Revert "IMPALA-9098: bump Impyla to 0.16.1"
     new 6b6e15b  IMPALA-9090: Add name of table being scanned in scan node profile
     new fc4a91c  IMPALA-9165: Add timeout for create-load-data.sh

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 be/src/exec/hbase-scan-node.cc                     |   1 +
 be/src/exec/hbase-table-writer.cc                  |   4 +-
 be/src/exec/hdfs-scan-node-base.cc                 |   1 +
 be/src/exec/kudu-scan-node-base.cc                 |   2 +
 bin/run-all-tests-timeout-check.sh                 |  70 -------------
 bin/run-all-tests.sh                               |   3 +-
 bin/script-timeout-check.sh                        | 111 +++++++++++++++++++++
 .../java/org/apache/impala/catalog/HBaseTable.java |   2 +-
 .../java/org/apache/impala/catalog/KuduTable.java  |   2 +-
 testdata/bin/create-load-data.sh                   |  16 +++
 10 files changed, 137 insertions(+), 75 deletions(-)
 delete mode 100755 bin/run-all-tests-timeout-check.sh
 create mode 100755 bin/script-timeout-check.sh


[impala] 01/02: IMPALA-9090: Add name of table being scanned in scan node profile

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

View the commit online:
https://github.com/apache/impala/commit/6b6e15b08ef799bbbb916c553b013dcd88ffb055

commit 6b6e15b08ef799bbbb916c553b013dcd88ffb055
Author: xiaomeng <xi...@cloudera.com>
AuthorDate: Mon Nov 4 14:46:59 2019 -0800

    IMPALA-9090: Add name of table being scanned in scan node profile
    
    Before this change, the only way to figure out the table being scanned
    by a scan node in the profile is to pull the string out of the explain
    plan or execsummary. This is awkward, both for manual and automated
    analysis of the profiles. We should include the table name as a string
    in the SCAN_NODE implementation.
    
    After this change, a new line "Table Name: database.table" will be added
    in first line of scan node profile.
    
    Also fix a bug that frontend pass hbase and kudu table name incorrectly
    to thrift. Before this change, native name of hbase and kudu table are
    passed in and there is no way to get hms table name from TableDescriptor
    in backend.
    
    After this change, for HBaseTableDescriptor and KuduTableDescriptor,
    function name() would return hms table name, function table_name()
    would return hbase or kudu native table name.
    
    Manually tested on mini-cluster with:
    1. hdfs and s3 table with file format text and parquet.
    2. hbase table.
    3. kudu table.
    
    Change-Id: If5da1112bcf38ae55b89eccfd7c7fad860819a99
    Reviewed-on: http://gerrit.cloudera.org:8080/14660
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/exec/hbase-scan-node.cc                             | 1 +
 be/src/exec/hbase-table-writer.cc                          | 4 ++--
 be/src/exec/hdfs-scan-node-base.cc                         | 1 +
 be/src/exec/kudu-scan-node-base.cc                         | 2 ++
 fe/src/main/java/org/apache/impala/catalog/HBaseTable.java | 2 +-
 fe/src/main/java/org/apache/impala/catalog/KuduTable.java  | 2 +-
 6 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/be/src/exec/hbase-scan-node.cc b/be/src/exec/hbase-scan-node.cc
index a794f45..c9380d8 100644
--- a/be/src/exec/hbase-scan-node.cc
+++ b/be/src/exec/hbase-scan-node.cc
@@ -115,6 +115,7 @@ Status HBaseScanNode::Prepare(RuntimeState* state) {
       sr.set_stop_key(key_range.stopKey);
     }
   }
+  runtime_profile_->AddInfoString("Table Name", hbase_table->fully_qualified_name());
   return Status::OK();
 }
 
diff --git a/be/src/exec/hbase-table-writer.cc b/be/src/exec/hbase-table-writer.cc
index 0120eab..9d75b7c 100644
--- a/be/src/exec/hbase-table-writer.cc
+++ b/be/src/exec/hbase-table-writer.cc
@@ -54,8 +54,8 @@ HBaseTableWriter::HBaseTableWriter(HBaseTableDescriptor* table_desc,
     runtime_profile_(profile) { }
 
 Status HBaseTableWriter::Init(RuntimeState* state) {
-  RETURN_IF_ERROR(ExecEnv::GetInstance()->htable_factory()->GetTable(table_desc_->name(),
-      &table_));
+  RETURN_IF_ERROR(ExecEnv::GetInstance()->htable_factory()->GetTable(
+      table_desc_->table_name(), &table_));
   encoding_timer_ = ADD_TIMER(runtime_profile_, "EncodingTimer");
   htable_put_timer_ = ADD_TIMER(runtime_profile_, "HTablePutTimer");
 
diff --git a/be/src/exec/hdfs-scan-node-base.cc b/be/src/exec/hdfs-scan-node-base.cc
index d81d817..e755ff6 100644
--- a/be/src/exec/hdfs-scan-node-base.cc
+++ b/be/src/exec/hdfs-scan-node-base.cc
@@ -270,6 +270,7 @@ Status HdfsScanNodeBase::Prepare(RuntimeState* state) {
   stringstream str;
   UpdateHdfsSplitStats(*scan_range_params_, &per_volume_stats);
   PrintHdfsSplitStats(per_volume_stats, &str);
+  runtime_profile()->AddInfoString("Table Name", hdfs_table_->fully_qualified_name());
   runtime_profile()->AddInfoString(HDFS_SPLIT_STATS_DESC, str.str());
   state->CheckAndAddCodegenDisabledMessage(runtime_profile());
   return Status::OK();
diff --git a/be/src/exec/kudu-scan-node-base.cc b/be/src/exec/kudu-scan-node-base.cc
index 35bd2c1..1bae536 100644
--- a/be/src/exec/kudu-scan-node-base.cc
+++ b/be/src/exec/kudu-scan-node-base.cc
@@ -107,6 +107,8 @@ Status KuduScanNodeBase::Open(RuntimeState* state) {
 
   KUDU_RETURN_IF_ERROR(client_->OpenTable(table_desc->table_name(), &table_),
       "Unable to open Kudu table");
+
+  runtime_profile_->AddInfoString("Table Name", table_desc->fully_qualified_name());
   return Status::OK();
 }
 
diff --git a/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java b/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
index 9e62459..4da0371 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
@@ -163,7 +163,7 @@ public class HBaseTable extends Table implements FeHBaseTable {
       Set<Long> referencedPartitions) {
     TTableDescriptor tableDescriptor =
         new TTableDescriptor(tableId, TTableType.HBASE_TABLE, getTColumnDescriptors(),
-            numClusteringCols_, hbaseTableName_, db_.getName());
+            numClusteringCols_, name_, db_.getName());
     tableDescriptor.setHbaseTable(Util.getTHBaseTable(this));
     return tableDescriptor;
   }
diff --git a/fe/src/main/java/org/apache/impala/catalog/KuduTable.java b/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
index af9b680..9c5b3a6 100644
--- a/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
@@ -463,7 +463,7 @@ public class KuduTable extends Table implements FeKuduTable {
   public TTableDescriptor toThriftDescriptor(int tableId,
       Set<Long> referencedPartitions) {
     TTableDescriptor desc = new TTableDescriptor(tableId, TTableType.KUDU_TABLE,
-        getTColumnDescriptors(), numClusteringCols_, kuduTableName_, db_.getName());
+        getTColumnDescriptors(), numClusteringCols_, name_, db_.getName());
     desc.setKuduTable(getTKuduTable());
     return desc;
   }


[impala] 02/02: IMPALA-9165: Add timeout for create-load-data.sh

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

View the commit online:
https://github.com/apache/impala/commit/fc4a91cf8c87966a910106dded7e7eb8d215270a

commit fc4a91cf8c87966a910106dded7e7eb8d215270a
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Mon Nov 18 18:01:14 2019 -0800

    IMPALA-9165: Add timeout for create-load-data.sh
    
    This converts the existing bin/run-all-tests-timeout-check.sh
    to a more generic bin/script-timeout-check.sh. It uses this
    new script for both bin/run-all-tests.sh and
    testdata/bin/create-load-data.sh. The new script takes two
    arguments:
     -timeout : timeout in minutes
     -script_name : name of the calling script
    The script_name is used in debugging output / output filenames
    to make it clear what timed out.
    
    The run-all-tests.sh timeout remains the same.
    testdata/bin/create-load-data.sh uses a 2.5 hour timeout.
    This should help debug the issue in IMPALA-9165, because at
    least the logs would be preserved on the Jenkins job.
    
    Testing:
     - Tested the timeout script by hand with a caller script that
       sleeps longer than the timeout
     - Ran a gerrit-verify-dryrun-external
    
    Change-Id: I19d76bd8850c7d4b5affff4d21f32d8715a382c6
    Reviewed-on: http://gerrit.cloudera.org:8080/14741
    Reviewed-by: Joe McDonnell <jo...@cloudera.com>
    Tested-by: Joe McDonnell <jo...@cloudera.com>
---
 bin/run-all-tests-timeout-check.sh |  70 -----------------------
 bin/run-all-tests.sh               |   3 +-
 bin/script-timeout-check.sh        | 111 +++++++++++++++++++++++++++++++++++++
 testdata/bin/create-load-data.sh   |  16 ++++++
 4 files changed, 129 insertions(+), 71 deletions(-)

diff --git a/bin/run-all-tests-timeout-check.sh b/bin/run-all-tests-timeout-check.sh
deleted file mode 100755
index dda9462..0000000
--- a/bin/run-all-tests-timeout-check.sh
+++ /dev/null
@@ -1,70 +0,0 @@
-#!/usr/bin/env bash
-#
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#   http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied.  See the License for the
-# specific language governing permissions and limitations
-# under the License.
-#
-# Script run by run-all-tests.sh that checks every 60 sec if the timeout expired and on
-# timeout prints the stacktraces of all impalads and then finally kills running tests.
-# Takes the timeout in minutes as an argument.
-
-SLEEP_TIMEOUT_S=0
-if [ -z "$1" ]; then
-  echo "Expected timeout value as an argument"
-  exit 1
-else
-  SLEEP_TIMEOUT_S=$(($1 * 60))
-fi
-
-[[ $SLEEP_TIMEOUT_S < 1 ]] && exit
-
-echo
-echo
-echo "**** Timout Timer Started (pid $$, ppid $PPID) for $SLEEP_TIMEOUT_S s! ****"
-echo
-echo
-
-# Check timer every 60 seconds and only proceed if the parent process is still alive.
-# Note: $SECONDS is a bash built-in that counts seconds since bash started.
-while ((SLEEP_TIMEOUT_S - SECONDS > 0)); do
-  sleep 1
-  if ! ps $PPID &> /dev/null; then
-    echo "Timeout Timer Exited because $PPID is gone."
-    exit
-  fi
-done
-
-echo
-echo
-echo '**** Tests TIMED OUT! ****'
-echo
-echo
-
-# Impala probably has a thread stuck. Print the stacktrace to the console output.
-mkdir -p "$IMPALA_TIMEOUT_LOGS_DIR"
-for pid in $(pgrep impalad); do
-  echo "**** Generating stacktrace of impalad with process id: $pid ****"
-  gdb -ex "thread apply all bt"  --batch -p $pid > "${IMPALA_TIMEOUT_LOGS_DIR}/${pid}.txt"
-done
-
-# Now kill any running tests.
-kill $PPID
-
-"${IMPALA_HOME}"/bin/generate_junitxml.py --step "test_run" --error "Test run timed out.
-This probably happened due to a hung thread which can be confirmed by looking at the
-stacktrace of running impalad processes at ${IMPALA_TIMEOUT_LOGS_DIR}"
-
-
diff --git a/bin/run-all-tests.sh b/bin/run-all-tests.sh
index bc849f0..6d4919b 100755
--- a/bin/run-all-tests.sh
+++ b/bin/run-all-tests.sh
@@ -121,7 +121,8 @@ do
   esac
 done
 
-"${IMPALA_HOME}/bin/run-all-tests-timeout-check.sh" $TIMEOUT_FOR_RUN_ALL_TESTS_MINS &
+"${IMPALA_HOME}/bin/script-timeout-check.sh" -timeout $TIMEOUT_FOR_RUN_ALL_TESTS_MINS \
+    -script_name "$(basename $0)" &
 TIMEOUT_PID=$!
 
 # IMPALA-3947: "Exhaustive" tests are actually based on workload. This
diff --git a/bin/script-timeout-check.sh b/bin/script-timeout-check.sh
new file mode 100755
index 0000000..ad5fd04
--- /dev/null
+++ b/bin/script-timeout-check.sh
@@ -0,0 +1,111 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+# Helper script that checks every 60 sec if the timeout expired and on timeout prints
+# the stacktraces of all impalads (if running) and then finally kills the caller script.
+# Takes two required arguments:
+# -script_name : name of the caller script (only for debug messages / output filenames)
+# -timeout : the timeout in minutes
+#
+# The way to use it is:
+# ${IMPALA_HOME}/bin/script-timeout-check.sh -timeout {TIMEOUT} -script_name {NAME} &
+# TIMEOUT_PID=$!
+# ... body of script ...
+# # Kill the spawned timeout process and its child sleep process. There may not be
+# # a sleep process, so ignore failure.
+# pkill -P $TIMEOUT_PID || true
+# kill $TIMEOUT_PID
+
+SCRIPT_NAME=""
+SLEEP_TIMEOUT_MIN=""
+
+# Parse commandline options
+while [ -n "$*" ]
+do
+  case $1 in
+    -timeout)
+        SLEEP_TIMEOUT_MIN=${2-}
+        shift;
+        ;;
+    -script_name)
+        SCRIPT_NAME=${2-}
+        shift;
+        ;;
+    -help|-h|*)
+        echo "script-timeout-check.sh : aborts caller script if timeout expires"
+        echo "[-timeout] : The timeout in minutes (required)"
+        echo "[-script_name] : The name of the caller script (required)"
+        exit 1;
+        ;;
+  esac
+  shift;
+done
+
+if [ -z "$SLEEP_TIMEOUT_MIN" ]; then
+  echo "Must pass a -timeout flag with a valid timeout as an argument"
+  exit 1
+fi;
+
+if [ -z "$SCRIPT_NAME" ]; then
+  echo "Must pass a -script_name flag with an appropriate argument"
+  exit 1
+fi;
+
+SLEEP_TIMEOUT_S=$((${SLEEP_TIMEOUT_MIN} * 60))
+
+[[ $SLEEP_TIMEOUT_S < 1 ]] && exit
+
+echo
+echo
+echo "**** Timeout Timer Started (pid $$, ppid $PPID) for $SLEEP_TIMEOUT_S s! ****"
+echo
+echo
+
+# Check timer every 60 seconds and only proceed if the parent process is still alive.
+# Note: $SECONDS is a bash built-in that counts seconds since bash started.
+while ((SLEEP_TIMEOUT_S - SECONDS > 0)); do
+  sleep 1
+  if ! ps $PPID &> /dev/null; then
+    echo "Timeout Timer Exited because $SCRIPT_NAME PID $PPID is gone."
+    exit
+  fi
+done
+
+echo
+echo
+echo "**** ${SCRIPT_NAME} TIMED OUT! ****"
+echo
+echo
+
+# Impala might have a thread stuck. Print the stacktrace to the console output.
+mkdir -p "$IMPALA_TIMEOUT_LOGS_DIR"
+for pid in $(pgrep impalad); do
+  echo "**** Generating stacktrace of impalad with process id: $pid ****"
+  gdb -ex "thread apply all bt"  --batch -p $pid > "${IMPALA_TIMEOUT_LOGS_DIR}/${pid}.txt"
+done
+
+# Now kill the caller
+kill $PPID
+
+"${IMPALA_HOME}"/bin/generate_junitxml.py --step "${SCRIPT_NAME}" \
+--error "Script ${SCRIPT_NAME} timed out. This probably happened due to a hung
+thread which can be confirmed by looking at the stacktrace of running impalad
+processes at ${IMPALA_TIMEOUT_LOGS_DIR}"
+
+
diff --git a/testdata/bin/create-load-data.sh b/testdata/bin/create-load-data.sh
index 6798567..6488851 100755
--- a/testdata/bin/create-load-data.sh
+++ b/testdata/bin/create-load-data.sh
@@ -43,6 +43,8 @@ setup_report_build_error
 : ${REMOTE_LOAD=}
 : ${CM_HOST=}
 : ${IMPALA_SERIAL_DATALOAD=}
+# We don't expect dataload to take more than 2.5 hours.
+: ${TIMEOUT_FOR_CREATE_LOAD_DATA_MINS:= 150}
 
 SKIP_METADATA_LOAD=0
 SKIP_SNAPSHOT_LOAD=0
@@ -90,6 +92,10 @@ do
     -skip_ranger)
       SKIP_RANGER=1
       ;;
+    -timeout)
+      TIMEOUT_FOR_CREATE_LOAD_DATA_MINS=${2-}
+      shift;
+      ;;
     -help|-h|*)
       echo "create-load-data.sh : Creates data and loads from scratch"
       echo "[-skip_metadata_load] : Skips loading of metadata"
@@ -97,6 +103,7 @@ do
       echo "[-snapshot_file] : Loads the test warehouse snapshot into hdfs"
       echo "[-cm_host] : Address of the Cloudera Manager host if loading to a remote cluster"
       echo "[-skip_ranger] : Skip the set-up for Ranger."
+      echo "[-timeout] : The timeout in minutes for loading data."
       exit 1;
       ;;
     esac
@@ -107,6 +114,10 @@ if [[ -n $REMOTE_LOAD ]]; then
   SKIP_RANGER=1
 fi
 
+"${IMPALA_HOME}/bin/script-timeout-check.sh" -timeout $TIMEOUT_FOR_CREATE_LOAD_DATA_MINS \
+    -script_name "$(basename $0)" &
+TIMEOUT_PID=$!
+
 if [[ $SKIP_METADATA_LOAD -eq 0  && "$SNAPSHOT_FILE" = "" ]]; then
   run-step "Generating HBase data" create-hbase.log \
       ${IMPALA_HOME}/testdata/bin/create-hbase.sh
@@ -704,3 +715,8 @@ fi
 # restarting the minicluster works and doesn't impact the tests. This is a common
 # operation for developers, so it is nice to test it.
 restart-cluster
+
+# Kill the spawned timeout process and its child sleep process.
+# There may not be a sleep process, so ignore failure.
+pkill -P $TIMEOUT_PID || true
+kill $TIMEOUT_PID