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 2020/09/01 23:31:20 UTC
[impala] branch master updated (69d0d0a -> 106dea6)
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 69d0d0a IMPALA-10087: IMPALA-6050 causes alluxio not to be supported
new 329bb41 IMPALA-10115: Impala should check file schema as well to check full ACIDv2 files
new 106dea6 IMPALA-10121: Generate JUnitXML for TSAN messages
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/hdfs-orc-scanner.cc | 8 ++---
be/src/exec/orc-metadata-utils.cc | 32 +++++++++---------
be/src/exec/orc-metadata-utils.h | 17 ++++++----
bin/jenkins/finalize.sh | 25 ++++++++++++--
testdata/data/README | 5 +++
.../data/full_acid_schema_but_no_acid_version.orc | Bin 0 -> 545 bytes
.../queries/QueryTest/acid-compaction.test | 37 +++++++++++++++++++++
tests/query_test/test_acid.py | 16 +++++++++
8 files changed, 111 insertions(+), 29 deletions(-)
create mode 100644 testdata/data/full_acid_schema_but_no_acid_version.orc
[impala] 02/02: IMPALA-10121: Generate JUnitXML for TSAN messages
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
commit 106dea63ba2f21ea43a580363445d4ad79a9c87c
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Tue Sep 1 09:40:16 2020 -0700
IMPALA-10121: Generate JUnitXML for TSAN messages
This adds logic in bin/jenkins/finalize.sh to check the ERROR
log for TSAN messages (i.e. WARNING: ThreadSanitizer: ...)
and generate a JUnitXML with the message. This happens when
TSAN aborts Impala.
Testing:
- Ran TSAN build (which is currently failing)
Change-Id: I44ea33a78482499decae0ec4c7c44513094b2f44
Reviewed-on: http://gerrit.cloudera.org:8080/16397
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
bin/jenkins/finalize.sh | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/bin/jenkins/finalize.sh b/bin/jenkins/finalize.sh
index 2c216e7..c8ca198 100755
--- a/bin/jenkins/finalize.sh
+++ b/bin/jenkins/finalize.sh
@@ -72,14 +72,35 @@ function check_for_asan_error {
fi
}
-# Check for AddressSanitizer messages. ASAN errors can show up in ERROR logs
-# (particularly for impalad). Some backend tests generate ERROR logs.
+function check_for_tsan_error {
+ ERROR_LOG=${1}
+ if grep -q "WARNING: ThreadSanitizer:" ${ERROR_LOG} ; then
+ # Extract out the TSAN message from the log file into a temp file.
+ # Starts with WARNING: ThreadSanitizer and then ends with a line with several '='
+ # characters (currently 18, we match 10).
+ tmp_tsan_output=$(mktemp)
+ sed -n '/ThreadSanitizer:/,/==========/p' ${ERROR_LOG} > "${tmp_tsan_output}"
+ # Make each TSAN issue use its own JUnitXML file by including the log filename
+ # in the step.
+ base=$(basename ${ERROR_LOG})
+ "${IMPALA_HOME}"/bin/generate_junitxml.py --phase finalize \
+ --step "tsan_error_${base}" \
+ --error "Thread Sanitizer message detected in ${ERROR_LOG}" \
+ --stderr "$(cat ${tmp_tsan_output})"
+ rm "${tmp_tsan_output}"
+ fi
+}
+
+# Check for AddressSanitizer/ThreadSanitizer messages. ASAN/TSAN errors can show up
+# in ERROR logs (particularly for impalad). Some backend tests generate ERROR logs.
for error_log in $(find $LOGS_DIR -name "*ERROR*"); do
check_for_asan_error ${error_log}
+ check_for_tsan_error ${error_log}
done
# Backend tests can also generate output in logs/be_tests/LastTest.log
if [[ -f ${LOGS_DIR}/be_tests/LastTest.log ]]; then
check_for_asan_error ${LOGS_DIR}/be_tests/LastTest.log
+ check_for_tsan_error ${LOGS_DIR}/be_tests/LastTest.log
fi
# Check for DCHECK messages. DCHECKs translate into CHECKs, which log at FATAL level
[impala] 01/02: IMPALA-10115: Impala should check file schema as
well to check full ACIDv2 files
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
commit 329bb41294a57bfd63dc0d90d57966e8562686b1
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Fri Aug 28 18:01:36 2020 +0200
IMPALA-10115: Impala should check file schema as well to check full ACIDv2 files
Currently Impala checks file metadata 'hive.acid.version' to decide the
full ACID schema. There are cases when Hive forgets to set this value
for full ACID files, e.g. query-based compactions.
So it's more robust to check the schema elements instead of the metadata
field. Also, sometimes Hive write the schema with different character
cases, e.g. originalTransaction vs originaltransaction, so we should
rather compare the column names in a case insensitive way.
Testing:
* added test for full ACID compaction
* added test_full_acid_schema_without_file_metadata_tag to test full
ACID file without metadata 'hive.acid.version'
Change-Id: I52642c1755599efd28fa2c90f13396cfe0f5fa14
Reviewed-on: http://gerrit.cloudera.org:8080/16383
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/exec/hdfs-orc-scanner.cc | 8 ++---
be/src/exec/orc-metadata-utils.cc | 32 +++++++++---------
be/src/exec/orc-metadata-utils.h | 17 ++++++----
testdata/data/README | 5 +++
.../data/full_acid_schema_but_no_acid_version.orc | Bin 0 -> 545 bytes
.../queries/QueryTest/acid-compaction.test | 37 +++++++++++++++++++++
tests/query_test/test_acid.py | 16 +++++++++
7 files changed, 88 insertions(+), 27 deletions(-)
diff --git a/be/src/exec/hdfs-orc-scanner.cc b/be/src/exec/hdfs-orc-scanner.cc
index c3e0baa..fe86013 100644
--- a/be/src/exec/hdfs-orc-scanner.cc
+++ b/be/src/exec/hdfs-orc-scanner.cc
@@ -190,8 +190,9 @@ Status HdfsOrcScanner::Open(ScannerContext* context) {
RETURN_IF_ERROR(footer_status);
bool is_table_full_acid = scan_node_->hdfs_table()->IsTableFullAcid();
- bool is_file_full_acid = reader_->hasMetadataValue(HIVE_ACID_VERSION_KEY) &&
- reader_->getMetadataValue(HIVE_ACID_VERSION_KEY) == "2";
+ schema_resolver_.reset(new OrcSchemaResolver(*scan_node_->hdfs_table(),
+ &reader_->getType(), filename(), is_table_full_acid));
+ bool is_file_full_acid = schema_resolver_->HasFullAcidV2Schema();
acid_original_file_ = is_table_full_acid && !is_file_full_acid;
if (is_table_full_acid) {
acid_write_id_range_ = valid_write_ids_.GetWriteIdRange(filename());
@@ -218,9 +219,6 @@ Status HdfsOrcScanner::Open(ScannerContext* context) {
filename()));
}
}
- schema_resolver_.reset(new OrcSchemaResolver(*scan_node_->hdfs_table(),
- &reader_->getType(), filename(), is_table_full_acid, is_file_full_acid));
- RETURN_IF_ERROR(schema_resolver_->ValidateFullAcidFileSchema());
// Hive Streaming Ingestion allocates multiple write ids, hence create delta directories
// like delta_5_10. Then it continuously appends new stripes (and footers) to the
diff --git a/be/src/exec/orc-metadata-utils.cc b/be/src/exec/orc-metadata-utils.cc
index aa81d7d..400bba0 100644
--- a/be/src/exec/orc-metadata-utils.cc
+++ b/be/src/exec/orc-metadata-utils.cc
@@ -17,9 +17,13 @@
#include "exec/orc-metadata-utils.h"
+#include <boost/algorithm/string.hpp>
+
#include "util/debug-util.h"
#include "common/names.h"
+using boost::algorithm::iequals;
+
namespace impala {
Status OrcSchemaResolver::BuildSchemaPaths(int num_partition_keys,
@@ -90,7 +94,6 @@ Status OrcSchemaResolver::ResolveColumn(const SchemaPath& col_path,
*node = root_;
*pos_field = false;
*missing_field = false;
- DCHECK_OK(ValidateFullAcidFileSchema()); // Should have already been validated.
if (col_path.empty()) return Status::OK();
SchemaPath table_path, file_path;
TranslateColPaths(col_path, &table_path, &file_path);
@@ -318,28 +321,27 @@ bool OrcSchemaResolver::IsAcidColumn(const SchemaPath& col_path) const {
col_path.front() >= num_part_cols && col_path.front() < num_part_cols + 5;
}
-Status OrcSchemaResolver::ValidateFullAcidFileSchema() const {
- if (!is_file_full_acid_) return Status::OK();
- string error_msg = Substitute("File %0 should have full ACID schema.", filename_);
- if (root_->getKind() != orc::TypeKind::STRUCT) return Status(error_msg);
- if (root_->getSubtypeCount() != 6) return Status(error_msg);
+void OrcSchemaResolver::DetermineFullAcidSchema() {
+ is_file_full_acid_ = false;
+ if (root_->getKind() != orc::TypeKind::STRUCT) return;
+ if (root_->getSubtypeCount() != 6) return;
if (root_->getSubtype(0)->getKind() != orc::TypeKind::INT ||
root_->getSubtype(1)->getKind() != orc::TypeKind::LONG ||
root_->getSubtype(2)->getKind() != orc::TypeKind::INT ||
root_->getSubtype(3)->getKind() != orc::TypeKind::LONG ||
root_->getSubtype(4)->getKind() != orc::TypeKind::LONG ||
root_->getSubtype(5)->getKind() != orc::TypeKind::STRUCT) {
- return Status(error_msg);
+ return;
}
- if (root_->getFieldName(0) != "operation" ||
- root_->getFieldName(1) != "originalTransaction" ||
- root_->getFieldName(2) != "bucket" ||
- root_->getFieldName(3) != "rowId" ||
- root_->getFieldName(4) != "currentTransaction" ||
- root_->getFieldName(5) != "row") {
- return Status(error_msg);
+ if (!iequals(root_->getFieldName(0), "operation") ||
+ !iequals(root_->getFieldName(1), "originaltransaction") ||
+ !iequals(root_->getFieldName(2), "bucket") ||
+ !iequals(root_->getFieldName(3), "rowid") ||
+ !iequals(root_->getFieldName(4), "currenttransaction") ||
+ !iequals(root_->getFieldName(5), "row")) {
+ return;
}
- return Status::OK();
+ is_file_full_acid_ = true;
}
} // namespace impala
diff --git a/be/src/exec/orc-metadata-utils.h b/be/src/exec/orc-metadata-utils.h
index b83b005..5450617 100644
--- a/be/src/exec/orc-metadata-utils.h
+++ b/be/src/exec/orc-metadata-utils.h
@@ -42,9 +42,10 @@ constexpr int CURRENT_TRANSCACTION_TYPE_ID = 5;
class OrcSchemaResolver {
public:
OrcSchemaResolver(const HdfsTableDescriptor& tbl_desc, const orc::Type* root,
- const char* filename, bool is_table_acid, bool is_file_acid) : tbl_desc_(tbl_desc),
- root_(root), filename_(filename), is_table_full_acid_(is_table_acid),
- is_file_full_acid_(is_file_acid) { }
+ const char* filename, bool is_table_acid) : tbl_desc_(tbl_desc), root_(root),
+ filename_(filename), is_table_full_acid_(is_table_acid) {
+ DetermineFullAcidSchema();
+ }
/// Resolve SchemaPath into orc::Type (ORC column representation)
/// 'pos_field' is set to true if 'col_path' reference the index field of an array
@@ -58,9 +59,8 @@ class OrcSchemaResolver {
Status BuildSchemaPaths(int num_partition_keys,
std::vector<SchemaPath>* col_id_path_map) const;
- /// Returns error if the file should be in ACIDv2 format,
- /// but the actual file schema doesn't conform to it.
- Status ValidateFullAcidFileSchema() const;
+ /// Returns true if file schema corresponds to full ACIDv2 format.
+ bool HasFullAcidV2Schema() const { return is_file_full_acid_; }
/// Can be only invoked for original files of full transactional tables.
/// Returns true if 'col_path' refers to an ACID column.
@@ -96,11 +96,14 @@ class OrcSchemaResolver {
SchemaPath GetCanonicalSchemaPath(const SchemaPath& col_path, int last_idx) const;
+ /// Sets 'is_file_full_acid_' based on the file schema.
+ void DetermineFullAcidSchema();
+
const HdfsTableDescriptor& tbl_desc_;
const orc::Type* const root_;
const char* const filename_ = nullptr;
const bool is_table_full_acid_;
- const bool is_file_full_acid_;
+ bool is_file_full_acid_;
/// Validate whether the ColumnType is compatible with the orc type
Status ValidateType(const ColumnType& type, const orc::Type& orc_type) const
diff --git a/testdata/data/README b/testdata/data/README
index 18c4ce9..2d5eb53 100644
--- a/testdata/data/README
+++ b/testdata/data/README
@@ -562,3 +562,8 @@ It contains the folloving lines:
1996-04-22 00:00:00
10:00:00.432100000
10:00:00
+
+full_acid_schema_but_no_acid_version.orc
+IMPALA-10115: Impala should check file schema as well to check full ACIDv2 files
+Genereted by query-based compaction by Hive 3.1. The file has full ACIDv2 schema, but
+doesn't have the file user metadata 'hive.acid.version'.
diff --git a/testdata/data/full_acid_schema_but_no_acid_version.orc b/testdata/data/full_acid_schema_but_no_acid_version.orc
new file mode 100644
index 0000000..91d15d7
Binary files /dev/null and b/testdata/data/full_acid_schema_but_no_acid_version.orc differ
diff --git a/testdata/workloads/functional-query/queries/QueryTest/acid-compaction.test b/testdata/workloads/functional-query/queries/QueryTest/acid-compaction.test
index 91ef3fa..a43c419 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/acid-compaction.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/acid-compaction.test
@@ -69,3 +69,40 @@ select * from upgraded_table;
2
3
====
+---- HIVE_QUERY
+use $DATABASE;
+create table full_acid (x int) stored as orc tblproperties('transactional'='true');
+
+insert into full_acid values (1);
+insert into full_acid values (2);
+insert into full_acid values (3);
+====
+---- QUERY
+invalidate metadata full_acid;
+select * from full_acid;
+---- RESULTS
+1
+2
+3
+====
+---- HIVE_QUERY
+use $DATABASE;
+alter table full_acid compact 'major' and wait;
+====
+---- QUERY
+refresh full_acid;
+select * from full_acid
+---- RESULTS
+1
+2
+3
+====
+---- QUERY
+show files in full_acid;
+---- LABELS
+Path,Size,Partition
+---- RESULTS
+row_regex:'$NAMENODE/$MANAGED_WAREHOUSE_DIR/$DATABASE.db/full_acid/base_0000003_v\d+/.*','\d+B',''
+---- TYPES
+STRING,STRING,STRING
+====
diff --git a/tests/query_test/test_acid.py b/tests/query_test/test_acid.py
index 416e1b9..55f6c56 100644
--- a/tests/query_test/test_acid.py
+++ b/tests/query_test/test_acid.py
@@ -314,3 +314,19 @@ class TestAcid(ImpalaTestSuite):
self._commit_txn(txn_id)
self.execute_query("refresh {}".format(tbl_name))
assert len(self.execute_query("select * from {}".format(tbl_name)).data) == 0
+
+ @SkipIfHive2.acid
+ def test_full_acid_schema_without_file_metadata_tag(self, vector, unique_database):
+ """IMPALA-10115: Some files have full ACID schema without having
+ 'hive.acid.version' set. We still need to identify such files as full ACID"""
+ table_name = "full_acid_schema_no_metadata"
+ fq_table_name = "{0}.{1}".format(unique_database, table_name)
+ self.client.execute("""CREATE TABLE {0} (i int) STORED AS ORC
+ TBLPROPERTIES('transactional'='true')""".format(fq_table_name))
+ table_uri = self._get_table_location(fq_table_name, vector)
+ acid_file = (os.environ['IMPALA_HOME'] +
+ "/testdata/data/full_acid_schema_but_no_acid_version.orc")
+ self.hdfs_client.copy_from_local(acid_file, table_uri + "/bucket_00000")
+ self.execute_query("refresh {}".format(fq_table_name))
+ result = self.execute_query("select count(*) from {0}".format(fq_table_name))
+ assert "3" in result.data