You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2017/10/10 15:13:33 UTC

[1/5] incubator-impala git commit: Download toolchain in parallel.

Repository: incubator-impala
Updated Branches:
  refs/heads/master 999a444e2 -> c189d0a39


Download toolchain in parallel.

By downloading from the toolchain S3 buckets in parallel with
extracting them, this improves bootstrap_toolchain on my machine
from about 1m5s to about 30s.

  $rm -rf toolchain; time bin/bootstrap_toolchain.py > /dev/null

  real    0m29.226s
  user    0m46.516s
  sys     0m33.820s

On a large EC2 machine, closer to the S3 buckets, the new time is 21s.

Because multiprocessing hasn't always been available (python2.4 on RHEL5
won't have it), I fall back to a simpler implementation

Change-Id: I46a6088bb002402c7653dbc8257dff869afb26ec
Reviewed-on: http://gerrit.cloudera.org:8080/8237
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Reviewed-by: Alex Behm <al...@cloudera.com>
Reviewed-by: Michael Brown <mi...@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/adb92d33
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/adb92d33
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/adb92d33

Branch: refs/heads/master
Commit: adb92d33979c71a62481e7eeae4b96f6588a18d5
Parents: 999a444
Author: Philip Zeyliger <ph...@cloudera.com>
Authored: Mon Oct 9 12:55:05 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Oct 10 01:25:27 2017 +0000

----------------------------------------------------------------------
 bin/bootstrap_toolchain.py | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/adb92d33/bin/bootstrap_toolchain.py
----------------------------------------------------------------------
diff --git a/bin/bootstrap_toolchain.py b/bin/bootstrap_toolchain.py
index 3a1f02d..76d4c5a 100755
--- a/bin/bootstrap_toolchain.py
+++ b/bin/bootstrap_toolchain.py
@@ -110,16 +110,17 @@ def bootstrap(toolchain_root, packages):
   # Detect the compiler
   compiler = "gcc-{0}".format(os.environ["IMPALA_GCC_VERSION"])
 
-  for p in packages:
+  def handle_package(p):
     pkg_name, pkg_version = unpack_name_and_version(p)
     if check_for_existing_package(toolchain_root, pkg_name, pkg_version, compiler):
-      continue
+      return
     if pkg_name != "kudu" or os.environ["KUDU_IS_SUPPORTED"] == "true":
       download_package(toolchain_root, pkg_name, pkg_version, compiler)
     else:
       build_kudu_stub(toolchain_root, pkg_version, compiler)
     write_version_file(toolchain_root, pkg_name, pkg_version, compiler,
         get_platform_release_label())
+  execute_many(handle_package, packages)
 
 def check_output(cmd_args):
   """Run the command and return the output. Raise an exception if the command returns
@@ -300,6 +301,22 @@ extern "C" void %s() {
   finally:
     shutil.rmtree(stub_build_dir)
 
+def execute_many(f, args):
+  """
+  Executes f(a) for a in args. If possible, uses a threadpool
+  to execute in parallel. The pool uses the number of CPUs
+  in the system as the default size.
+  """
+  pool = None
+  try:
+    import multiprocessing.pool
+    pool = multiprocessing.pool.ThreadPool()
+    return pool.map(f, args, 1)
+  except ImportError:
+    # multiprocessing was introduced in Python 2.6.
+    # For older Pythons (CentOS 5), degrade to single-threaded execution:
+    return [ f(a) for a in args ]
+
 def download_cdh_components(toolchain_root, cdh_components):
   """Downloads and unpacks the CDH components into $CDH_COMPONENTS_HOME if not found."""
   cdh_components_home = os.getenv("CDH_COMPONENTS_HOME")
@@ -315,17 +332,20 @@ def download_cdh_components(toolchain_root, cdh_components):
   # The URL prefix of where CDH components live in S3.
   download_path_prefix = HOST + "/cdh_components/"
 
-  for component in cdh_components:
+
+  def download(component):
     pkg_name, pkg_version = unpack_name_and_version(component)
     pkg_directory = package_directory(cdh_components_home, pkg_name, pkg_version)
     if os.path.isdir(pkg_directory):
-      continue
+      return
 
     # Download the package if it doesn't exist
     file_name = "{0}-{1}.tar.gz".format(pkg_name, pkg_version)
     download_path = download_path_prefix + file_name
     wget_and_unpack_package(download_path, file_name, cdh_components_home, False)
 
+  execute_many(download, cdh_components)
+
 if __name__ == "__main__":
   """Validates the presence of $IMPALA_HOME and $IMPALA_TOOLCHAIN in the environment.-
   By checking $IMPALA_HOME is present, we assume that IMPALA_{LIB}_VERSION will be present
@@ -350,9 +370,12 @@ if __name__ == "__main__":
   if not os.path.exists(toolchain_root):
     os.makedirs(toolchain_root)
 
-  packages = ["avro", "binutils", "boost", "breakpad", "bzip2", "cmake", "crcutil",
-      "flatbuffers", "gcc", "gflags", "glog", "gperftools", "gtest", "kudu", "libev",
-      "llvm", ("llvm", "3.9.1-asserts"), "lz4", "openldap", "openssl", "protobuf",
+  # LLVM and Kudu are the largest packages. Sort them first so that
+  # their download starts as soon as possible.
+  packages = ["llvm", ("llvm", "3.9.1-asserts"), "kudu",
+      "avro", "binutils", "boost", "breakpad", "bzip2", "cmake", "crcutil",
+      "flatbuffers", "gcc", "gflags", "glog", "gperftools", "gtest", "libev",
+      "lz4", "openldap", "openssl", "protobuf",
       "rapidjson", "re2", "snappy", "thrift", "tpc-h", "tpc-ds", "zlib"]
   bootstrap(toolchain_root, packages)
 


[5/5] incubator-impala git commit: IMPALA-6016: Fix logging in TableLoadingMgr class

Posted by mi...@apache.org.
IMPALA-6016: Fix logging in TableLoadingMgr class

This patch moves the logging of "loads in progress"
to a place where the current load is accounted. The
reason to move the logging is that the current load
is not reflected in the loadingTables_ till loadAsync()
is called.

Change-Id: I925a6ba9a09be25df2759da5e6d85dfc8b981ce4
Reviewed-on: http://gerrit.cloudera.org:8080/8212
Reviewed-by: Bharath Vissapragada <bh...@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/c189d0a3
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c189d0a3
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c189d0a3

Branch: refs/heads/master
Commit: c189d0a39cfb106f369522c57926196cc772da5e
Parents: e570343
Author: Bharath Vissapragada <bh...@cloudera.com>
Authored: Wed Oct 4 15:30:03 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Oct 10 02:45:46 2017 +0000

----------------------------------------------------------------------
 .../java/org/apache/impala/catalog/TableLoadingMgr.java  | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c189d0a3/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java b/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
index f3abfee..d07c0ed 100644
--- a/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
+++ b/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
@@ -67,6 +67,10 @@ public class TableLoadingMgr {
     public Table get() {
       Table tbl;
       try {
+        LOG.info("Loading metadata for table: " +
+            tblName_.db_name + "." + tblName_.table_name);
+        LOG.info(String.format("Remaining items in queue: %s. Loads in progress: %s",
+            tableLoadingDeque_.size(), loadingTables_.size()));
         tbl = tblTask_.get();
       } catch (Exception e) {
         tbl = IncompleteTable.createFailedMetadataLoadTable(
@@ -282,15 +286,12 @@ public class TableLoadingMgr {
   private void loadNextTable() throws InterruptedException {
     // Always get the next table from the head of the deque.
     final TTableName tblName = tableLoadingDeque_.takeFirst();
-    LOG.info("Loading next table from queue: " +
-        tblName.db_name + "." + tblName.table_name);
-    LOG.info(String.format("Remaining items in queue: %s. Loads in progress: %s",
-        tableLoadingDeque_.size(), loadingTables_.size()));
-
     AtomicBoolean isLoading = tableLoadingBarrier_.get(tblName);
     if (isLoading == null || !isLoading.compareAndSet(false, true)) {
       // Another thread has already completed the load or the load is still in progress.
       // Return so this thread can work on another table in the queue.
+      LOG.info("Metadata load request already in progress for table: " +
+          tblName.db_name + "." + tblName.table_name);
       return;
     }
     try {


[4/5] incubator-impala git commit: IMPALA-4682: Remove Preconditions check from analyzeAggregation().

Posted by mi...@apache.org.
IMPALA-4682: Remove Preconditions check from analyzeAggregation().

When one runs a query like

'select * from t order by count(a)'

we are incorrectly throwing an IllegalStateException, with a
Preconditions check which asserts that the column is not "*". Now,
this query is invalid, and we are correctly handling it if "*" is
replaced by a specific column:

'select a from t order by count(b)'

which produces the error message

"ERROR: AnalysisException: select list expression not produced by
aggregation output (missing from GROUP BY clause?): a"

This patch fixes the handling of "*" in this context, by removing
the Preconditions check, so that the error becomes

"ERROR: AnalysisException: select list expression not produced by
aggregation output (missing from GROUP BY clause?): *"

Note that the second changed line is required because
selectListItem.Expr_ is null when SelectListItem.isStar_ is true.

A new FE unit test has been added for this use case.

Change-Id: I57c20aeed401275d45913fedfd61c206c38641b7
Reviewed-on: http://gerrit.cloudera.org:8080/8143
Reviewed-by: Alex Behm <al...@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/e5703433
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/e5703433
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/e5703433

Branch: refs/heads/master
Commit: e5703433200007e634d7d9fdc895a1dbbdd4b3f4
Parents: 2747d52
Author: Zoram Thanga <zo...@cloudera.com>
Authored: Tue Sep 26 13:27:38 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Oct 10 02:37:42 2017 +0000

----------------------------------------------------------------------
 fe/src/main/java/org/apache/impala/analysis/SelectStmt.java       | 3 +--
 fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java | 3 +++
 2 files changed, 4 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e5703433/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
index c146247..068e09a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
@@ -725,11 +725,10 @@ public class SelectStmt extends QueryStmt {
     for (int i = 0; i < selectList_.getItems().size(); ++i) {
       if (!resultExprs_.get(i).isBound(finalAggInfo.getOutputTupleId())) {
         SelectListItem selectListItem = selectList_.getItems().get(i);
-        Preconditions.checkState(!selectListItem.isStar());
         throw new AnalysisException(
             "select list expression not produced by aggregation output "
             + "(missing from GROUP BY clause?): "
-            + selectListItem.getExpr().toSql());
+            + selectListItem.toSql());
       }
     }
     if (orderByElements_ != null) {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e5703433/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
index b22ca81..1032d07 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
@@ -1940,6 +1940,9 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
     AnalysisError("select functional.alltypes.*, max(string_col) from " +
         "functional.alltypes", "cannot combine '*' in select list with grouping or " +
         "aggregation");
+    AnalysisError("select * from functional.alltypes order by count(*)",
+        "select list expression not produced by aggregation output " +
+        "(missing from GROUP BY clause?): *");
 
     // only count() allows '*'
     AnalysisError("select avg(*) from functional.testtbl",


[2/5] incubator-impala git commit: IMPALA-5448: fix invalid number of splits reported in Parquet scan node

Posted by mi...@apache.org.
IMPALA-5448: fix invalid number of splits reported in Parquet scan node

Parquet splits with multi columns are marked as completed by using
HdfsScanNodeBase::RangeComplete(). It duplicately counts the file types
as column codec types. Thus the number of parquet splits are the real count
multiplies number of materialized columns.

Furthermore, according to the Parquet definition, it allows mixed compression
codecs on different columns. This's handled in this patch as well. A parquet file
using gzip and snappy compression codec will be reported as:
	FileFormats: PARQUET/(GZIP,SNAPPY):1

This patch introduces a compression types set for the above cases.

Testing:
Add end-to-end tests handling parquet files with all columns compressed in
snappy, and handling parquet files with multi compression codec.

Change-Id: Iaacc2d775032f5707061e704f12e0a63cde695d1
Reviewed-on: http://gerrit.cloudera.org:8080/8147
Reviewed-by: Tim Armstrong <ta...@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/192cd96d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/192cd96d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/192cd96d

Branch: refs/heads/master
Commit: 192cd96d9ee3be1cd3e7c3ad774bf8d5c8efb1c0
Parents: adb92d3
Author: stiga-huang <hu...@gmail.com>
Authored: Tue Sep 26 16:45:11 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Oct 10 01:30:33 2017 +0000

----------------------------------------------------------------------
 be/src/exec/hdfs-scan-node-base.cc              |  28 ++++++++--
 be/src/exec/hdfs-scan-node-base.h               |  56 ++++++++++++++++---
 testdata/multi_compression_parquet_data/README  |   8 +++
 .../tinytable_0_gzip_snappy.parq                | Bin 0 -> 573 bytes
 .../tinytable_1_snappy_gzip.parq                | Bin 0 -> 578 bytes
 .../hdfs_parquet_scan_node_profile.test         |  20 +++++++
 tests/query_test/test_scanners.py               |  33 +++++++++++
 7 files changed, 132 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/192cd96d/be/src/exec/hdfs-scan-node-base.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-scan-node-base.cc b/be/src/exec/hdfs-scan-node-base.cc
index 59d4ca2..1cfee5e 100644
--- a/be/src/exec/hdfs-scan-node-base.cc
+++ b/be/src/exec/hdfs-scan-node-base.cc
@@ -697,9 +697,11 @@ void HdfsScanNodeBase::RangeComplete(const THdfsFileFormat::type& file_type,
     const vector<THdfsCompression::type>& compression_types, bool skipped) {
   scan_ranges_complete_counter()->Add(1);
   progress_.Update(1);
+  HdfsCompressionTypesSet compression_set;
   for (int i = 0; i < compression_types.size(); ++i) {
-    ++file_type_counts_[std::make_tuple(file_type, skipped, compression_types[i])];
+    compression_set.AddType(compression_types[i]);
   }
+  ++file_type_counts_[std::make_tuple(file_type, skipped, compression_set)];
 }
 
 void HdfsScanNodeBase::ComputeSlotMaterializationOrder(vector<int>* order) const {
@@ -786,19 +788,33 @@ void HdfsScanNodeBase::StopAndFinalizeCounters() {
 
         THdfsFileFormat::type file_format = std::get<0>(it->first);
         bool skipped = std::get<1>(it->first);
-        THdfsCompression::type compression_type = std::get<2>(it->first);
+        HdfsCompressionTypesSet compressions_set = std::get<2>(it->first);
+        int file_cnt = it->second;
 
         if (skipped) {
           if (file_format == THdfsFileFormat::PARQUET) {
             // If a scan range stored as parquet is skipped, its compression type
             // cannot be figured out without reading the data.
-            ss << file_format << "/" << "Unknown" << "(Skipped):" << it->second << " ";
+            ss << file_format << "/" << "Unknown" << "(Skipped):" << file_cnt << " ";
           } else {
-            ss << file_format << "/" << compression_type << "(Skipped):"
-               << it->second << " ";
+            ss << file_format << "/" << compressions_set.GetFirstType() << "(Skipped):"
+               << file_cnt << " ";
           }
+        } else if (compressions_set.Size() == 1) {
+          ss << file_format << "/" << compressions_set.GetFirstType() << ":" << file_cnt
+             << " ";
         } else {
-          ss << file_format << "/" << compression_type << ":" << it->second << " ";
+          ss << file_format << "/" << "(";
+          bool first = true;
+          for (auto& elem : _THdfsCompression_VALUES_TO_NAMES) {
+            THdfsCompression::type type = static_cast<THdfsCompression::type>(
+                elem.first);
+            if (!compressions_set.HasType(type)) continue;
+            if (!first) ss << ",";
+            ss << type;
+            first = false;
+          }
+          ss << "):" << file_cnt << " ";
         }
       }
     }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/192cd96d/be/src/exec/hdfs-scan-node-base.h
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-scan-node-base.h b/be/src/exec/hdfs-scan-node-base.h
index da95ecc..252964b 100644
--- a/be/src/exec/hdfs-scan-node-base.h
+++ b/be/src/exec/hdfs-scan-node-base.h
@@ -479,13 +479,6 @@ class HdfsScanNodeBase : public ScanNode {
   /// scanner threads.
   Status status_;
 
-  /// Mapping of file formats (file type, compression type) to the number of
-  /// splits of that type and the lock protecting it.
-  typedef std::map<
-     std::tuple<THdfsFileFormat::type, bool, THdfsCompression::type>,
-     int> FileTypeCountsMap;
-  FileTypeCountsMap file_type_counts_;
-
   /// Performs dynamic partition pruning, i.e., applies runtime filters to files, and
   /// issues initial ranges for all file types. Waits for runtime filters if necessary.
   /// Only valid to call if !initial_ranges_issued_. Sets initial_ranges_issued_ to true.
@@ -525,6 +518,55 @@ class HdfsScanNodeBase : public ScanNode {
   /// Calls ExecNode::ExecDebugAction() with 'phase'. Returns the status based on the
   /// debug action specified for the query.
   Status ScanNodeDebugAction(TExecNodePhase::type phase) WARN_UNUSED_RESULT;
+
+ private:
+  class HdfsCompressionTypesSet {
+   public:
+    HdfsCompressionTypesSet(): bit_map_(0) {
+      DCHECK_GE(sizeof(bit_map_) * CHAR_BIT, _THdfsCompression_VALUES_TO_NAMES.size());
+    }
+
+    bool HasType(THdfsCompression::type type) {
+      return (bit_map_ & (1 << type)) != 0;
+    }
+
+    void AddType(const THdfsCompression::type type) {
+      bit_map_ |= (1 << type);
+    }
+
+    int Size() { return BitUtil::Popcount(bit_map_); }
+
+    THdfsCompression::type GetFirstType() {
+      DCHECK_GT(Size(), 0);
+      for (auto& elem : _THdfsCompression_VALUES_TO_NAMES) {
+        THdfsCompression::type type = static_cast<THdfsCompression::type>(elem.first);
+        if (HasType(type))  return type;
+      }
+      return THdfsCompression::NONE;
+    }
+
+    // The following operator overloading is needed so this class can be part of the
+    // std::map key.
+    bool operator< (const HdfsCompressionTypesSet& o) const {
+      return bit_map_ < o.bit_map_;
+    }
+
+    bool operator== (const HdfsCompressionTypesSet& o) const {
+      return bit_map_ == o.bit_map_;
+    }
+
+   private:
+    uint32_t bit_map_;
+  };
+
+  /// Mapping of file formats to the number of splits of that type. The key is a tuple
+  /// containing:
+  /// * file type
+  /// * whether the split was skipped
+  /// * compression types set
+  typedef std::map<std::tuple<THdfsFileFormat::type, bool, HdfsCompressionTypesSet>, int>
+      FileTypeCountsMap;
+  FileTypeCountsMap file_type_counts_;
 };
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/192cd96d/testdata/multi_compression_parquet_data/README
----------------------------------------------------------------------
diff --git a/testdata/multi_compression_parquet_data/README b/testdata/multi_compression_parquet_data/README
new file mode 100644
index 0000000..332f24d
--- /dev/null
+++ b/testdata/multi_compression_parquet_data/README
@@ -0,0 +1,8 @@
+These Parquet files were created by modifying Impala's HdfsParquetTableWriter.
+
+String Data
+-----------
+These files have two string columns 'a' and 'b'. Each columns using different compression types.
+
+tinytable_0_gzip_snappy.parq: column 'a' is compressed by gzip, column 'b' is compressed by snappy
+tinytable_1_gzip_snappy.parq: column 'a' is compressed by snappy, column 'b' is compressed by gzip

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/192cd96d/testdata/multi_compression_parquet_data/tinytable_0_gzip_snappy.parq
----------------------------------------------------------------------
diff --git a/testdata/multi_compression_parquet_data/tinytable_0_gzip_snappy.parq b/testdata/multi_compression_parquet_data/tinytable_0_gzip_snappy.parq
new file mode 100644
index 0000000..4fd468b
Binary files /dev/null and b/testdata/multi_compression_parquet_data/tinytable_0_gzip_snappy.parq differ

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/192cd96d/testdata/multi_compression_parquet_data/tinytable_1_snappy_gzip.parq
----------------------------------------------------------------------
diff --git a/testdata/multi_compression_parquet_data/tinytable_1_snappy_gzip.parq b/testdata/multi_compression_parquet_data/tinytable_1_snappy_gzip.parq
new file mode 100644
index 0000000..e5d2220
Binary files /dev/null and b/testdata/multi_compression_parquet_data/tinytable_1_snappy_gzip.parq differ

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/192cd96d/testdata/workloads/functional-query/queries/QueryTest/hdfs_parquet_scan_node_profile.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/hdfs_parquet_scan_node_profile.test b/testdata/workloads/functional-query/queries/QueryTest/hdfs_parquet_scan_node_profile.test
new file mode 100644
index 0000000..7e55ab1
--- /dev/null
+++ b/testdata/workloads/functional-query/queries/QueryTest/hdfs_parquet_scan_node_profile.test
@@ -0,0 +1,20 @@
+# Regression test for IMPALA-5448
+# This query will do a full scan on a parquet file
+select * from functional_parquet.alltypestiny where year=2009 and month=1
+---- RUNTIME_PROFILE
+row_regex: .*File Formats: PARQUET/SNAPPY:1
+====
+---- QUERY
+# This query will do a full scan on a parquet table with two partitions.
+# Each partition uses different compression types.
+select * from alltypes_multi_compression
+---- RUNTIME_PROFILE
+row_regex: .*File Formats: PARQUET/GZIP:1 PARQUET/SNAPPY:1
+====
+---- QUERY
+# This query will do a full scan on a parquet table with multiple
+# compression types
+select * from multi_compression
+---- RUNTIME_PROFILE
+row_regex: .*File Formats: PARQUET/\(GZIP,SNAPPY\):2
+====

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/192cd96d/tests/query_test/test_scanners.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_scanners.py b/tests/query_test/test_scanners.py
index f4f2fd6..ca7cd6b 100644
--- a/tests/query_test/test_scanners.py
+++ b/tests/query_test/test_scanners.py
@@ -327,6 +327,39 @@ class TestParquet(ImpalaTestSuite):
     assert len(result.data) == 1
     assert "4294967294" in result.data
 
+  def test_multi_compression_types(self, vector, unique_database):
+    """IMPALA-5448: Tests that parquet splits with multi compression types are counted
+    correctly. Cases tested:
+    - parquet file with columns using the same compression type
+    - parquet files using snappy and gzip compression types
+    """
+    self.client.execute("create table %s.alltypes_multi_compression like"
+        " functional_parquet.alltypes" % unique_database)
+    hql_format = "set parquet.compression={codec};" \
+        "insert into table %s.alltypes_multi_compression" \
+        "  partition (year = {year}, month = {month})" \
+        "  select id, bool_col, tinyint_col, smallint_col, int_col, bigint_col," \
+        "    float_col, double_col,date_string_col,string_col,timestamp_col" \
+        "  from functional_parquet.alltypes" \
+        "  where year = {year} and month = {month}" % unique_database
+    check_call(['hive', '-e', hql_format.format(codec="snappy", year=2010, month=1)])
+    check_call(['hive', '-e', hql_format.format(codec="gzip", year=2010, month=2)])
+
+    self.client.execute("create table %s.multi_compression (a string, b string)"
+        " stored as parquet" % unique_database)
+    multi_compression_tbl_loc =\
+        get_fs_path("/test-warehouse/%s.db/%s" % (unique_database, "multi_compression"))
+    check_call(['hdfs', 'dfs', '-copyFromLocal', os.environ['IMPALA_HOME'] +
+        "/testdata/multi_compression_parquet_data/tinytable_0_gzip_snappy.parq",
+        multi_compression_tbl_loc])
+    check_call(['hdfs', 'dfs', '-copyFromLocal', os.environ['IMPALA_HOME'] +
+        "/testdata/multi_compression_parquet_data/tinytable_1_snappy_gzip.parq",
+        multi_compression_tbl_loc])
+
+    vector.get_value('exec_option')['num_nodes'] = 1
+    self.run_test_case('QueryTest/hdfs_parquet_scan_node_profile',
+                       vector, unique_database)
+
   def test_corrupt_rle_counts(self, vector, unique_database):
     """IMPALA-3646: Tests that a certain type of file corruption for plain
     dictionary encoded values is gracefully handled. Cases tested:


[3/5] incubator-impala git commit: IMPALA-5940: Avoid log spew by using Status::Expected.

Posted by mi...@apache.org.
IMPALA-5940: Avoid log spew by using Status::Expected.

In IMPALA-5926, we fixed a case where closing the session triggered a
stack trace in the logs which impacted performance for short-running
queries. Looking at log files from several active clusters, I identified a few
other cases where we could clean up log spew with the same (trivial)
approach.

The following messages will no longer log:

  "Failed to codegen MaterializeTuple() due to unsupported type: $0"
  "Not implemented for this format."
  "ScalarFnCall Codegen not supported for CHAR"
  "Could not codegen CodegenMaterializeExprs: "
  "Could not codegen TupleRowComparator::Compare(): $0"

These codegen related messages were happening at every execution node, and were
therefore very common.

In addition, the following messages, which were very frequent in the logs, now
will log, but without the back trace:

  "Query Id ... not found."

To do this, I changed them from logging implicitly via Status(...)
to logging explicitly.

I have not fixed this message:
  "... Client ... timed-out during recv call."
It's in DoRpc(), and not all callers of DoRpc() have independent
logging. For now, we'll trade a little bit of log spam for clearer
debugging.

The snippet I used to identify these was:

  find . -type f -name '*IMPALAD*.gz' | xargs gzcat  | awk '/^I/ { if(x) { print x; } x = "" } /status.cc/ { x=" "; } { if(x) { x=x  $0 } }'  | sed -e 's/0x[0-9a-fx]* //g' | sed -e 's/[0-9a-f]\{16\}:[0-9a-f]*/QUERYID/g' |  tr -s '\t' ' ' | tr '[0-9]' 'N' | sort | uniq -c  | sort -n | tee output.txt

I also analyzed some logs using SQL, against a pre-processed logs table:

  with v as (
    select regexp_replace(
        regexp_replace(
          translate(substr(message, 42), "\n\t", "  "),
          "[a-zA-Z0-9.-]*[.][a-zA-Z0-9-]*:[0-9]*",
          "<host>"),
        "@.*$", "@@@...") as m
    from logs_table where `class`="status.cc")
  select m, count(*) from v group by 1 order by 2 desc limit 100

Testing:
* Automated tests.
* Manual testing for one of the new back-trace-suppressed paths:

  $ impala-python shell/gen-py/ImpalaService/ImpalaService-remote -h localhost:21000 GetRuntimeProfile 'beeswaxd.ttypes.QueryHandle()'
  Traceback (most recent call last):
    File "shell/gen-py/ImpalaService/ImpalaService-remote", line 106, in <module>
      pp.pprint(client.GetRuntimeProfile(eval(args[0]),))
    File "/home/philip/src/impala/shell/gen-py/ImpalaService/ImpalaService.py", line 161, in GetRuntimeProfile
      return self.recv_GetRuntimeProfile()
    File "/home/philip/src/impala/shell/gen-py/ImpalaService/ImpalaService.py", line 184, in recv_GetRuntimeProfile
      raise result.error
  beeswaxd.ttypes.BeeswaxException: BeeswaxException(handle=QueryHandle(log_context='', id=''), log_context='', SQLState='HY000', _message='GetRuntimeProfile error: Query id 0:0 not found.\n', errorCode=0)

  $ grep 'Query id' logs/cluster/impalad.INFO | tail -n 1
  I0926 20:29:09.999944  6787 impala-server.cc:642] Query id 0:0 not found.

Change-Id: I38088482377a1c3e794a9c8178ef83f29957a330
Reviewed-on: http://gerrit.cloudera.org:8080/8100
Reviewed-by: Tim Armstrong <ta...@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/2747d521
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/2747d521
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/2747d521

Branch: refs/heads/master
Commit: 2747d521ad92ea38f3cf25fee6de6777dbf95677
Parents: 192cd96
Author: Philip Zeyliger <ph...@cloudera.com>
Authored: Thu Sep 14 20:35:30 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Oct 10 02:20:49 2017 +0000

----------------------------------------------------------------------
 be/src/exec/hdfs-avro-scanner.cc   |  2 +-
 be/src/exec/hdfs-scan-node-base.cc |  2 +-
 be/src/exprs/scalar-fn-call.cc     |  4 ++--
 be/src/runtime/tuple.cc            |  5 ++---
 be/src/service/impala-server.cc    | 14 ++++++++------
 be/src/util/tuple-row-compare.cc   |  4 ++--
 6 files changed, 16 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2747d521/be/src/exec/hdfs-avro-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-avro-scanner.cc b/be/src/exec/hdfs-avro-scanner.cc
index 6771e63..b6883ac 100644
--- a/be/src/exec/hdfs-avro-scanner.cc
+++ b/be/src/exec/hdfs-avro-scanner.cc
@@ -969,7 +969,7 @@ Status HdfsAvroScanner::CodegenReadScalar(const AvroSchemaElement& element,
       read_field_fn = codegen->GetFunction(IRFunction::READ_AVRO_DECIMAL, false);
       break;
     default:
-      return Status(Substitute(
+      return Status::Expected(Substitute(
           "Failed to codegen MaterializeTuple() due to unsupported type: $0",
           element.schema->type));
   }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2747d521/be/src/exec/hdfs-scan-node-base.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-scan-node-base.cc b/be/src/exec/hdfs-scan-node-base.cc
index 1cfee5e..1c6a4de 100644
--- a/be/src/exec/hdfs-scan-node-base.cc
+++ b/be/src/exec/hdfs-scan-node-base.cc
@@ -307,7 +307,7 @@ void HdfsScanNodeBase::Codegen(RuntimeState* state) {
       default:
         // No codegen for this format
         fn = NULL;
-        status = Status("Not implemented for this format.");
+        status = Status::Expected("Not implemented for this format.");
     }
     DCHECK(fn != NULL || !status.ok());
     const char* format_name = _THdfsFileFormat_VALUES_TO_NAMES.find(format)->second;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2747d521/be/src/exprs/scalar-fn-call.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/scalar-fn-call.cc b/be/src/exprs/scalar-fn-call.cc
index 82b886c..20dbe07 100644
--- a/be/src/exprs/scalar-fn-call.cc
+++ b/be/src/exprs/scalar-fn-call.cc
@@ -296,12 +296,12 @@ Status ScalarFnCall::GetCodegendComputeFn(LlvmCodeGen* codegen, Function** fn) {
     return Status::OK();
   }
   if (type_.type == TYPE_CHAR) {
-    return Status("ScalarFnCall Codegen not supported for CHAR");
+    return Status::Expected("ScalarFnCall Codegen not supported for CHAR");
   }
   for (int i = 0; i < GetNumChildren(); ++i) {
     if (children_[i]->type().type == TYPE_CHAR) {
       *fn = NULL;
-      return Status("ScalarFnCall Codegen not supported for CHAR");
+      return Status::Expected("ScalarFnCall Codegen not supported for CHAR");
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2747d521/be/src/runtime/tuple.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/tuple.cc b/be/src/runtime/tuple.cc
index 252175f..1dab5a5 100644
--- a/be/src/runtime/tuple.cc
+++ b/be/src/runtime/tuple.cc
@@ -305,9 +305,8 @@ Status Tuple::CodegenMaterializeExprs(LlvmCodeGen* codegen, bool collect_string_
     Status status = slot_materialize_exprs[i]->GetCodegendComputeFn(codegen,
         &materialize_expr_fns[i]);
     if (!status.ok()) {
-      stringstream ss;
-      ss << "Could not codegen CodegenMaterializeExprs: " << status.GetDetail();
-      return Status(ss.str());
+      return Status::Expected(Substitute("Could not codegen CodegenMaterializeExprs: $0",
+            status.GetDetail()));
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2747d521/be/src/service/impala-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index fabc8fa..9574fb8 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -636,9 +636,10 @@ Status ImpalaServer::GetRuntimeProfileStr(const TUniqueId& query_id,
     lock_guard<mutex> l(query_log_lock_);
     QueryLogIndex::const_iterator query_record = query_log_index_.find(query_id);
     if (query_record == query_log_index_.end()) {
-      stringstream ss;
-      ss << "Query id " << PrintId(query_id) << " not found.";
-      return Status(ss.str());
+      // Common error, so logging explicitly and eliding Status's stack trace.
+      string err = strings::Substitute("Query id $0 not found.", PrintId(query_id));
+      VLOG(1) << err;
+      return Status::Expected(err);
     }
     RETURN_IF_ERROR(CheckProfileAccess(user, query_record->second->effective_user,
         query_record->second->user_has_profile_access));
@@ -691,9 +692,10 @@ Status ImpalaServer::GetExecSummary(const TUniqueId& query_id, const string& use
       }
     }
     if (is_query_missing) {
-      stringstream ss;
-      ss << "Query id " << PrintId(query_id) << " not found.";
-      return Status(ss.str());
+      // Common error, so logging explicitly and eliding Status's stack trace.
+      string err = strings::Substitute("Query id $0 not found.", PrintId(query_id));
+      VLOG(1) << err;
+      return Status::Expected(err);
     }
     RETURN_IF_ERROR(CheckProfileAccess(user, effective_user, user_has_profile_access));
     *result = exec_summary;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2747d521/be/src/util/tuple-row-compare.cc
----------------------------------------------------------------------
diff --git a/be/src/util/tuple-row-compare.cc b/be/src/util/tuple-row-compare.cc
index 9a012ca..3d43576 100644
--- a/be/src/util/tuple-row-compare.cc
+++ b/be/src/util/tuple-row-compare.cc
@@ -211,8 +211,8 @@ Status TupleRowComparator::CodegenCompare(LlvmCodeGen* codegen, Function** fn) {
   for (int i = 0; i < ordering_exprs.size(); ++i) {
     Status status = ordering_exprs[i]->GetCodegendComputeFn(codegen, &key_fns[i]);
     if (!status.ok()) {
-      return Status(Substitute("Could not codegen TupleRowComparator::Compare(): $0",
-          status.GetDetail()));
+      return Status::Expected(Substitute(
+            "Could not codegen TupleRowComparator::Compare(): $0", status.GetDetail()));
     }
   }