You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by pr...@apache.org on 2025/02/21 16:51:26 UTC
(impala) branch branch-4.5.0 updated (7dd93d461 -> f52de8740)
This is an automated email from the ASF dual-hosted git repository.
prozsa pushed a change to branch branch-4.5.0
in repository https://gitbox.apache.org/repos/asf/impala.git
discard 7dd93d461 Release 4.5.0
new 72044cbaa IMPALA-13759: Fix Hive ACID INSERT OVERWRITE base detection
new fdcf7a144 IMPALA-13771: Fix heap-use-after-free in Cluster Membership Manager
new 442ec1d31 IMPALA-13768: Redundant Iceberg delete records are shuffled around which cause error "Invalid file path arrived at builder"
new 732ec3bb9 IMPALA-13770: Updating Iceberg tables with UDFs crashes Impala
new f52de8740 Release 4.5.0
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (7dd93d461)
\
N -- N -- N refs/heads/branch-4.5.0 (f52de8740)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 5 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/iceberg-delete-builder.cc | 21 ++++++++-------
be/src/exec/iceberg-merge-node.cc | 7 +++++
be/src/exec/iceberg-merge-sink.cc | 8 +++++-
be/src/exec/iceberg-merge-sink.h | 2 ++
be/src/exec/multi-table-sink.cc | 7 +++++
be/src/exec/multi-table-sink.h | 2 ++
be/src/runtime/krpc-data-stream-sender.cc | 22 +++++++--------
be/src/scheduling/cluster-membership-mgr.cc | 5 +++-
bin/save-version.sh | 2 +-
.../java/org/apache/impala/util/AcidUtils.java | 20 ++++++++++++--
.../java/org/apache/impala/util/AcidUtilsTest.java | 18 +++++++++++++
.../QueryTest/iceberg-delete-partitioned.test | 30 +++++++++++++++++++++
.../queries/QueryTest/iceberg-merge.test | 31 +++++++++++++++++++++-
.../queries/QueryTest/iceberg-update-basic.test | 25 +++++++++++++++++
tests/query_test/test_acid.py | 25 +++++++++++++++++
tests/query_test/test_iceberg.py | 7 +++--
16 files changed, 202 insertions(+), 30 deletions(-)
(impala) 03/05: IMPALA-13768: Redundant Iceberg delete records are shuffled around which cause error "Invalid file path arrived at builder"
Posted by pr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
prozsa pushed a commit to branch branch-4.5.0
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 442ec1d3168014d651e68f40762952eb8dc31ac5
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Wed Feb 19 14:11:19 2025 +0100
IMPALA-13768: Redundant Iceberg delete records are shuffled around which cause error "Invalid file path arrived at builder"
IcebergDeleteBuilder assumes that it should only receive delete
records for paths of data files that are scheduled for its
corresponding SCAN operator.
It is not true when any of the following happens:
* number of output channels in sender is 1
(currently no DIRECTED mode, no filtering)
* hit bug in DIRECTED mode, see below
* single node plan is being used (no DIRECTED mode, no filtering)
With this patch, KrpcDataStreamSender::Send() will use DIRECTED mode
even if number of output channels is 1. It also fixes the bug in
DIRECTED mode (which was due to an unused variable 'skipped_prev_row')
and simplified the logic a bit.
The patch also relaxes the assumption in IcebergDeleteBuilder, i.e.
only return error for dangling delete records when we are in a
distributed plan where we can assume DIRECTED distribution mode of
position delete records.
Testing
* added e2e tests
Change-Id: I695c919c9a74edec768e413a02b2ef7dbfa0d6a5
Reviewed-on: http://gerrit.cloudera.org:8080/22500
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/exec/iceberg-delete-builder.cc | 21 ++++++++-------
be/src/runtime/krpc-data-stream-sender.cc | 22 +++++++---------
.../QueryTest/iceberg-delete-partitioned.test | 30 ++++++++++++++++++++++
3 files changed, 51 insertions(+), 22 deletions(-)
diff --git a/be/src/exec/iceberg-delete-builder.cc b/be/src/exec/iceberg-delete-builder.cc
index 4692aca0e..fc4833ace 100644
--- a/be/src/exec/iceberg-delete-builder.cc
+++ b/be/src/exec/iceberg-delete-builder.cc
@@ -261,16 +261,19 @@ Status IcebergDeleteBuilder::AddToDeletedRows(const StringValue& path,
} else if (path.Len() == 0) {
return Status("NULL found as file_path in delete file");
} else {
- // 'deleted_rows_' should contain all data file paths that this join
- // processes.
- stringstream ss;
- ss << "Invalid file path arrived at builder: " << path << " Paths expected: [";
- for (auto& [vec_path, unused] : deleted_rows_) {
- ss << vec_path << ", ";
+ // We received a path of a data file that is not scheduled for this fragment.
+ if (is_separate_build_) {
+ // When the build is in its own fragment we assume that delete records are filtered
+ // in the sender by DIRECTED distribution mode. Therefore we should never see
+ // dangling delete records.
+ stringstream ss;
+ ss << "Invalid file path arrived at builder: " << path << " Paths expected: [";
+ for (auto& [vec_path, unused] : deleted_rows_) {
+ ss << vec_path << ", ";
+ }
+ ss << "]";
+ return Status(ss.str());
}
- ss << "]";
- DCHECK(false) << ss.str();
- return Status(ss.str());
}
return Status::OK();
}
diff --git a/be/src/runtime/krpc-data-stream-sender.cc b/be/src/runtime/krpc-data-stream-sender.cc
index 6a41018c2..127ced2a1 100644
--- a/be/src/runtime/krpc-data-stream-sender.cc
+++ b/be/src/runtime/krpc-data-stream-sender.cc
@@ -1133,7 +1133,8 @@ Status KrpcDataStreamSender::Send(RuntimeState* state, RowBatch* batch) {
}
// At this point no RPCs can still refer to the old in_flight_batch_.
in_flight_batch_.swap(serialization_batch_);
- } else if (partition_type_ == TPartitionType::RANDOM || channels_.size() == 1) {
+ } else if (partition_type_ == TPartitionType::RANDOM ||
+ (channels_.size() == 1 && partition_type_ != TPartitionType::DIRECTED)) {
// Round-robin batches among channels. Wait for the current channel to finish its
// rpc before overwriting its batch.
Channel* current_channel = channels_[current_channel_idx_].get();
@@ -1171,7 +1172,6 @@ Status KrpcDataStreamSender::Send(RuntimeState* state, RowBatch* batch) {
const int num_rows = batch->num_rows();
char* prev_filename_ptr = nullptr;
vector<IcebergPositionDeleteChannel*> prev_channels;
- bool skipped_prev_row = false;
for (int row_idx = 0; row_idx < num_rows; ++row_idx) {
DCHECK_EQ(batch->num_tuples_per_row(), 1);
TupleRow* tuple_row = batch->GetRow(row_idx);
@@ -1182,24 +1182,22 @@ Status KrpcDataStreamSender::Send(RuntimeState* state, RowBatch* batch) {
if (filename_value_ss.ptr == prev_filename_ptr) {
// If the filename pointer is the same as the previous one then we can instantly
// send the row to the same channels as the previous row.
- DCHECK(skipped_prev_row || !prev_channels.empty() ||
- (filename_value_ss.len == 0 && prev_channels.empty()));
for (IcebergPositionDeleteChannel* ch : prev_channels) {
RETURN_IF_ERROR(ch->AddRow(tuple_row));
}
continue;
}
+ prev_channels.clear();
prev_filename_ptr = filename_value_ss.ptr;
string filename(filename_value_ss.ptr, filename_value_ss.len);
const auto filepath_to_hosts_it = filepath_to_hosts_.find(filename);
if (filepath_to_hosts_it == filepath_to_hosts_.end()) {
- // This can happen when e.g. compaction removed some data files from a snapshot
- // but a delete file referencing them remained because it references other data
- // files that remains in the new snapshot.
- // Another use-case is table sampling where we read only a subset of the data
- // files.
- // A third case is when the delete record is invalid.
+ // This can happen due to file pruning, or when compaction removed some data
+ // files from a snapshot but a delete file referencing them remained because
+ // it references other data files that are still present in the new snapshot.
+ // Another case is table sampling where we read only a subset of the data files.
+ // Or, when simply the delete record is invalid.
if (UNLIKELY(filename_value_ss.len == 0)) {
state->LogError(
ErrorMsg(TErrorCode::GENERAL, "NULL found as file_path in delete file"));
@@ -1207,12 +1205,10 @@ Status KrpcDataStreamSender::Send(RuntimeState* state, RowBatch* batch) {
VLOG(3) << "Row from delete file refers to a non-existing data file: " <<
filename;
}
- skipped_prev_row = true;
+ DCHECK(prev_channels.empty());
continue;
}
- skipped_prev_row = false;
- prev_channels.clear();
for (const NetworkAddressPB& host_addr : filepath_to_hosts_it->second) {
const auto channel_map_it = host_to_channel_.find(host_addr);
if (channel_map_it == host_to_channel_.end()) {
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-partitioned.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-partitioned.test
index 0e8d8e973..b81fa89c8 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-partitioned.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-partitioned.test
@@ -803,3 +803,33 @@ delete from ice_alltypes_part_v2 where i=1;
---- TYPES
INT, BOOLEAN, INT, BIGINT, FLOAT, DOUBLE, DECIMAL, DATE, STRING
====
+---- QUERY
+# Create table to test IMPALA-13768. It will have multiple partitions, with
+# multiple data files in each partition, where min/max ranges of column 'bi'
+# is disjunct for data files (in same partition).
+create table ice_invalid_deletes (bi bigint, year int)
+partitioned by spec (year)
+stored as iceberg tblproperties ('format-version'='2');
+insert into ice_invalid_deletes
+ select bigint_col, year from functional.alltypes where month = 10;
+with v as (select max(bi) as max_bi from ice_invalid_deletes)
+insert into ice_invalid_deletes select bi + v.max_bi, year from v, ice_invalid_deletes;
+delete from ice_invalid_deletes where bi % 11 = 0;
+====
+---- QUERY
+# Scan single data file (single channel in EXCHANGE sender) with delete file
+# that have references to other data files.
+select count(*) from ice_invalid_deletes where year=2010 and bi = 180;
+---- RESULTS
+31
+---- TYPES
+BIGINT
+====
+---- QUERY
+# Scan single data file per partition with delete files that reference other data files.
+select count(*) from ice_invalid_deletes where year>2000 and bi = 180;
+---- RESULTS
+62
+---- TYPES
+BIGINT
+====
(impala) 04/05: IMPALA-13770: Updating Iceberg tables with UDFs crashes Impala
Posted by pr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
prozsa pushed a commit to branch branch-4.5.0
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 732ec3bb959c0ff649134eda4bbf82c07cba23ba
Author: Daniel Becker <da...@cloudera.com>
AuthorDate: Thu Feb 20 09:21:05 2025 +0100
IMPALA-13770: Updating Iceberg tables with UDFs crashes Impala
When using a native UDF in the target value of an UPDATE statement or in
a filter predicate or target value of a MERGE statement, Impala crashes
with the following DCHECK:
be/src/exprs/expr.cc:47 47 DCHECK(cache_entry_ == nullptr);
This DCHECK is in the destructor of Expr, and it fires because Close()
has not been called for the expression. In the UPDATE case this is
caused by MultiTableSinkConfig: it creates child DataSinkConfig objects
but does not call Close() on them, and consequently these child sink
configs do not call Close() on their output expressions.
In the MERGE case it is because various expressions are not closed in
IcebergMergeCase and IcebergMergeNode.
This patch fixes the issue by overriding Close() in MultiTableSinkConfig,
calling Close() on the child sinks as well as closing the expressions in
IcebergMergeCase and IcebergMergeNode.
Testing:
- Added EE regression tests for the UPDATE and MERGE cases in
iceberg-update-basic.test and iceberg-merge.test
Change-Id: Id86638c8d6d86062c68cc9d708ec9c7b0a4e95eb
Reviewed-on: http://gerrit.cloudera.org:8080/22508
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/exec/iceberg-merge-node.cc | 7 +++++
be/src/exec/iceberg-merge-sink.cc | 8 +++++-
be/src/exec/iceberg-merge-sink.h | 2 ++
be/src/exec/multi-table-sink.cc | 7 +++++
be/src/exec/multi-table-sink.h | 2 ++
.../queries/QueryTest/iceberg-merge.test | 31 +++++++++++++++++++++-
.../queries/QueryTest/iceberg-update-basic.test | 25 +++++++++++++++++
tests/query_test/test_iceberg.py | 7 +++--
8 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/be/src/exec/iceberg-merge-node.cc b/be/src/exec/iceberg-merge-node.cc
index f591ead46..0c3409bc9 100644
--- a/be/src/exec/iceberg-merge-node.cc
+++ b/be/src/exec/iceberg-merge-node.cc
@@ -283,6 +283,11 @@ void IcebergMergeNode::Close(RuntimeState* state) {
row_present_evaluator_->Close(state);
ScalarExprEvaluator::Close(position_meta_evaluators_, state);
ScalarExprEvaluator::Close(partition_meta_evaluators_, state);
+
+ row_present_->Close();
+ ScalarExpr::Close(position_meta_exprs_);
+ ScalarExpr::Close(partition_meta_exprs_);
+
ExecNode::Close(state);
}
@@ -326,6 +331,8 @@ Status IcebergMergeCase::Open(RuntimeState* state) {
void IcebergMergeCase::Close(RuntimeState* state) {
ScalarExprEvaluator::Close(filter_evaluators_, state);
ScalarExprEvaluator::Close(output_evaluators_, state);
+ ScalarExpr::Close(output_exprs_);
+ ScalarExpr::Close(filter_conjuncts_);
}
} // namespace impala
diff --git a/be/src/exec/iceberg-merge-sink.cc b/be/src/exec/iceberg-merge-sink.cc
index 00ade9363..0cf6f224c 100644
--- a/be/src/exec/iceberg-merge-sink.cc
+++ b/be/src/exec/iceberg-merge-sink.cc
@@ -65,6 +65,12 @@ DataSink* IcebergMergeSinkConfig::CreateSink(RuntimeState* state) const {
return state->obj_pool()->Add(new IcebergMergeSink(sink_id, *this, *tsink_, state));
}
+void IcebergMergeSinkConfig::Close() {
+ delete_sink_config_->Close();
+ insert_sink_config_->Close();
+ DataSinkConfig::Close();
+}
+
IcebergMergeSink::IcebergMergeSink(TDataSinkId sink_id,
const IcebergMergeSinkConfig& sink_config, const TDataSink& dsink,
RuntimeState* state)
@@ -137,4 +143,4 @@ void IcebergMergeSink::Close(RuntimeState* state) {
DCHECK(closed_);
}
-} // namespace impala
\ No newline at end of file
+} // namespace impala
diff --git a/be/src/exec/iceberg-merge-sink.h b/be/src/exec/iceberg-merge-sink.h
index e5a3c2dd3..3863b3891 100644
--- a/be/src/exec/iceberg-merge-sink.h
+++ b/be/src/exec/iceberg-merge-sink.h
@@ -46,6 +46,8 @@ class IcebergMergeSinkConfig : public DataSinkConfig {
ScalarExpr* merge_action() const { return merge_action_; }
+ virtual void Close() override;
+
protected:
Status Init(const TDataSink& tsink, const RowDescriptor* input_row_desc,
FragmentState* state) override;
diff --git a/be/src/exec/multi-table-sink.cc b/be/src/exec/multi-table-sink.cc
index c37e9cf2d..81f158c25 100644
--- a/be/src/exec/multi-table-sink.cc
+++ b/be/src/exec/multi-table-sink.cc
@@ -44,6 +44,13 @@ DataSink* MultiTableSinkConfig::CreateSink(RuntimeState* state) const {
new MultiTableSink(sink_id, *this, *tsink_, state));
}
+void MultiTableSinkConfig::Close() {
+ for (TableSinkBaseConfig* table_sink_config : table_sink_configs_) {
+ table_sink_config->Close();
+ }
+ DataSinkConfig::Close();
+}
+
MultiTableSink::MultiTableSink(TDataSinkId sink_id,
const MultiTableSinkConfig& sink_config, const TDataSink& dsink,
RuntimeState* state) : DataSink(sink_id, sink_config, "MultiTableSink", state) {
diff --git a/be/src/exec/multi-table-sink.h b/be/src/exec/multi-table-sink.h
index 05ca124f4..125663818 100644
--- a/be/src/exec/multi-table-sink.h
+++ b/be/src/exec/multi-table-sink.h
@@ -39,6 +39,8 @@ class MultiTableSinkConfig : public DataSinkConfig {
return table_sink_configs_;
}
+ virtual void Close() override;
+
~MultiTableSinkConfig() override {}
protected:
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-merge.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-merge.test
index 3e82301de..cd1a4f390 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-merge.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-merge.test
@@ -138,4 +138,33 @@ when not matched then insert *
6,true,6,6.599999904632568,60,2009-01-01,'6'
---- TYPES
INT,BOOLEAN,INT,FLOAT,DECIMAL,DATE,STRING
-====
\ No newline at end of file
+====
+
+---- QUERY
+# Regression test for IMPALA-13770: using a native UDF as a filter predicate and target
+# value of a MERGE statement.
+# Note: even with the incorrect behaviour before IMPALA-13770 the test may pass and the
+# values can also be updated successfully. This is because the crash happens after the
+# query is closed, when releasing resources. This test is a valid regression test because
+# even if it passes, there will be a crash: some later queries are likely to fail and
+# there will be a minidump in the build artifacts, so the build will be marked FAILED.
+create function if not exists identity(int) returns int location 'UDF_LOCATION' symbol='Identity';
+create table merge_with_udf(int_col INT)
+stored by iceberg
+tblproperties ('format-version'='2');
+insert into merge_with_udf values (1), (2);
+====
+---- QUERY
+merge into merge_with_udf t
+using merge_with_udf s on s.int_col = t.int_col
+when matched and s.int_col != identity(t.int_col) + 1
+ then update set int_col = identity(s.int_col)
+---- DML_RESULTS: merge_with_udf
+1
+2
+---- TYPES
+INT
+---- RUNTIME_PROFILE
+NumModifiedRows: 2
+NumDeletedRows: 2
+====
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-update-basic.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-update-basic.test
index f3e2c81c6..2bcf91cd8 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-update-basic.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-update-basic.test
@@ -404,3 +404,28 @@ row_regex:'$NAMENODE/test-warehouse/$DATABASE.db/ice_part_transforms/data/ts_day
---- TYPES
STRING, STRING, STRING, STRING
====
+
+---- QUERY
+# Regression test for IMPALA-13770: using a native UDF in the target value of an UPDATE
+# Note: even with the incorrect behaviour before IMPALA-13770 the test may pass and the
+# values can also be updated successfully. This is because the crash happens after the
+# query is closed, when releasing resources. This test is a valid regression test because
+# even if it passes, there will be a crash: some later queries are likely to fail and
+# there will be a minidump in the build artifacts, so the build will be marked FAILED.
+create function if not exists identity(int) returns int location 'UDF_LOCATION' symbol='Identity';
+create table update_with_udf(int_col INT)
+stored by iceberg
+tblproperties ('format-version'='2');
+insert into update_with_udf values (1), (2);
+====
+---- QUERY
+update update_with_udf set int_col = identity(int_col);
+---- DML_RESULTS: update_with_udf
+1
+2
+---- TYPES
+INT
+---- RUNTIME_PROFILE
+NumModifiedRows: 2
+NumDeletedRows: 2
+====
diff --git a/tests/query_test/test_iceberg.py b/tests/query_test/test_iceberg.py
index ab51bab13..697f7117d 100644
--- a/tests/query_test/test_iceberg.py
+++ b/tests/query_test/test_iceberg.py
@@ -1688,8 +1688,9 @@ class TestIcebergV2Table(IcebergTestSuite):
assert hive_output == "id\n4\n5\n6\n7\n8\n"
def test_update_basic(self, vector, unique_database):
+ udf_location = get_fs_path('/test-warehouse/libTestUdfs.so')
self.run_test_case('QueryTest/iceberg-update-basic', vector,
- unique_database)
+ unique_database, test_file_vars={'UDF_LOCATION': udf_location})
self._test_update_basic_snapshots(unique_database)
if IS_HDFS and self.should_run_for_hive(vector):
self._update_basic_hive_tests(unique_database)
@@ -1987,7 +1988,9 @@ class TestIcebergV2Table(IcebergTestSuite):
self._check_file_filtering(tbl_name, 100, "REWRITE_ALL", True)
def test_merge(self, vector, unique_database):
- self.run_test_case('QueryTest/iceberg-merge', vector, unique_database)
+ udf_location = get_fs_path('/test-warehouse/libTestUdfs.so')
+ self.run_test_case('QueryTest/iceberg-merge', vector, unique_database,
+ test_file_vars={'UDF_LOCATION': udf_location})
def test_merge_partition(self, vector, unique_database):
self.run_test_case('QueryTest/iceberg-merge-partition', vector, unique_database)
(impala) 05/05: Release 4.5.0
Posted by pr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
prozsa pushed a commit to branch branch-4.5.0
in repository https://gitbox.apache.org/repos/asf/impala.git
commit f52de8740257937fee4a593970c4733798b4b7ed
Author: Peter Rozsa <pr...@cloudera.com>
AuthorDate: Wed Jan 29 11:06:35 2025 +0100
Release 4.5.0
Change-Id: Idb2fe419f645415cb1768095a0c537de75add13d
---
bin/impala-config.sh | 2 +-
bin/save-version.sh | 2 +-
fe/pom.xml | 2 +-
java/TableFlattener/pom.xml | 2 +-
java/calcite-planner/pom.xml | 6 +++---
java/datagenerator/pom.xml | 2 +-
java/executor-deps/pom.xml | 2 +-
java/ext-data-source/api/pom.xml | 2 +-
java/ext-data-source/jdbc/pom.xml | 2 +-
java/ext-data-source/pom.xml | 2 +-
java/ext-data-source/sample/pom.xml | 2 +-
java/ext-data-source/test/pom.xml | 2 +-
java/external-frontend/pom.xml | 4 ++--
java/pom.xml | 2 +-
java/puffin-data-generator/pom.xml | 2 +-
java/query-event-hook-api/pom.xml | 2 +-
java/shaded-deps/hive-exec/pom.xml | 2 +-
java/shaded-deps/s3a-aws-sdk/pom.xml | 2 +-
java/test-corrupt-hive-udfs/pom.xml | 2 +-
java/test-hive-udfs/pom.xml | 2 +-
java/yarn-extras/pom.xml | 2 +-
21 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/bin/impala-config.sh b/bin/impala-config.sh
index d7d0fdf36..904870533 100755
--- a/bin/impala-config.sh
+++ b/bin/impala-config.sh
@@ -70,7 +70,7 @@ fi
# WARNING: If changing this value, also run these commands:
# cd ${IMPALA_HOME}/java
# mvn versions:set -DnewVersion=YOUR_NEW_VERSION
-export IMPALA_VERSION=4.5.0-SNAPSHOT
+export IMPALA_VERSION=4.5.0-RELEASE
# Whether to build the backend on Avro C++ library or C.
# This is added temporarily to help transitioning from Avro C to C++ library.
diff --git a/bin/save-version.sh b/bin/save-version.sh
index 85e5f899e..6093f08bf 100755
--- a/bin/save-version.sh
+++ b/bin/save-version.sh
@@ -22,7 +22,7 @@
# "-INTERNAL" appended. Parts of the code will look for this to distinguish
# between released and internal versions.
VERSION=${IMPALA_VERSION}
-GIT_HASH=$(git rev-parse HEAD 2> /dev/null)
+GIT_HASH=732ec3bb959c0ff649134eda4bbf82c07cba23ba
if [ -z $GIT_HASH ]
then
GIT_HASH="Could not obtain git hash"
diff --git a/fe/pom.xml b/fe/pom.xml
index 450f27280..05f5b0a58 100644
--- a/fe/pom.xml
+++ b/fe/pom.xml
@@ -23,7 +23,7 @@ under the License.
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
<relativePath>../java/pom.xml</relativePath>
</parent>
<modelVersion>4.0.0</modelVersion>
diff --git a/java/TableFlattener/pom.xml b/java/TableFlattener/pom.xml
index b7d0d28d0..7c5abaaa5 100644
--- a/java/TableFlattener/pom.xml
+++ b/java/TableFlattener/pom.xml
@@ -22,7 +22,7 @@
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
<artifactId>nested-table-flattener</artifactId>
diff --git a/java/calcite-planner/pom.xml b/java/calcite-planner/pom.xml
index e00925046..a97d53b67 100644
--- a/java/calcite-planner/pom.xml
+++ b/java/calcite-planner/pom.xml
@@ -22,12 +22,12 @@ under the License.
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
<artifactId>calcite-planner</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
<packaging>jar</packaging>
<name>calcite-planner</name>
@@ -36,7 +36,7 @@ under the License.
<dependency>
<groupId>org.apache.impala</groupId>
<artifactId>impala-frontend</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</dependency>
<dependency>
<groupId>org.apache.calcite</groupId>
diff --git a/java/datagenerator/pom.xml b/java/datagenerator/pom.xml
index b6e8f3b3f..d34b16b0f 100644
--- a/java/datagenerator/pom.xml
+++ b/java/datagenerator/pom.xml
@@ -23,7 +23,7 @@ under the License.
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
diff --git a/java/executor-deps/pom.xml b/java/executor-deps/pom.xml
index 62a31d54d..6c6c3c958 100644
--- a/java/executor-deps/pom.xml
+++ b/java/executor-deps/pom.xml
@@ -34,7 +34,7 @@ under the License.
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
<groupId>org.apache.impala</groupId>
diff --git a/java/ext-data-source/api/pom.xml b/java/ext-data-source/api/pom.xml
index 32c4b8a4c..8ec23cdbd 100644
--- a/java/ext-data-source/api/pom.xml
+++ b/java/ext-data-source/api/pom.xml
@@ -23,7 +23,7 @@
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-data-source</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<artifactId>impala-data-source-api</artifactId>
<name>Apache Impala External Data Source API</name>
diff --git a/java/ext-data-source/jdbc/pom.xml b/java/ext-data-source/jdbc/pom.xml
index 6b39daf96..f9658cba0 100644
--- a/java/ext-data-source/jdbc/pom.xml
+++ b/java/ext-data-source/jdbc/pom.xml
@@ -23,7 +23,7 @@
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-data-source</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<artifactId>impala-data-source-jdbc</artifactId>
<name>Apache Impala External Data Source JDBC Library</name>
diff --git a/java/ext-data-source/pom.xml b/java/ext-data-source/pom.xml
index 87e25f207..16797631a 100644
--- a/java/ext-data-source/pom.xml
+++ b/java/ext-data-source/pom.xml
@@ -22,7 +22,7 @@
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
<artifactId>impala-data-source</artifactId>
diff --git a/java/ext-data-source/sample/pom.xml b/java/ext-data-source/sample/pom.xml
index 6f3860a4f..6148e1a45 100644
--- a/java/ext-data-source/sample/pom.xml
+++ b/java/ext-data-source/sample/pom.xml
@@ -23,7 +23,7 @@
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-data-source</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<artifactId>impala-data-source-sample</artifactId>
<name>Apache Impala External Data Source Sample</name>
diff --git a/java/ext-data-source/test/pom.xml b/java/ext-data-source/test/pom.xml
index a25728e03..292fc447f 100644
--- a/java/ext-data-source/test/pom.xml
+++ b/java/ext-data-source/test/pom.xml
@@ -23,7 +23,7 @@
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-data-source</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<artifactId>impala-data-source-test</artifactId>
<name>Apache Impala External Data Source Test Library</name>
diff --git a/java/external-frontend/pom.xml b/java/external-frontend/pom.xml
index 2b46b8a8c..416eb26d3 100644
--- a/java/external-frontend/pom.xml
+++ b/java/external-frontend/pom.xml
@@ -22,7 +22,7 @@ under the License.
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
@@ -36,7 +36,7 @@ under the License.
<dependency>
<groupId>org.apache.impala</groupId>
<artifactId>impala-frontend</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</dependency>
</dependencies>
diff --git a/java/pom.xml b/java/pom.xml
index ae5752532..89a126a22 100644
--- a/java/pom.xml
+++ b/java/pom.xml
@@ -21,7 +21,7 @@ under the License.
<modelVersion>4.0.0</modelVersion>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
<packaging>pom</packaging>
<name>Apache Impala Parent POM</name>
diff --git a/java/puffin-data-generator/pom.xml b/java/puffin-data-generator/pom.xml
index 9688b1f6f..5d4780768 100644
--- a/java/puffin-data-generator/pom.xml
+++ b/java/puffin-data-generator/pom.xml
@@ -23,7 +23,7 @@ under the License.
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
diff --git a/java/query-event-hook-api/pom.xml b/java/query-event-hook-api/pom.xml
index ce74b4391..510f44058 100644
--- a/java/query-event-hook-api/pom.xml
+++ b/java/query-event-hook-api/pom.xml
@@ -22,7 +22,7 @@
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
<artifactId>query-event-hook-api</artifactId>
diff --git a/java/shaded-deps/hive-exec/pom.xml b/java/shaded-deps/hive-exec/pom.xml
index 18be87df6..d8c8e2547 100644
--- a/java/shaded-deps/hive-exec/pom.xml
+++ b/java/shaded-deps/hive-exec/pom.xml
@@ -27,7 +27,7 @@ the same dependencies
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
<relativePath>../../pom.xml</relativePath>
</parent>
<modelVersion>4.0.0</modelVersion>
diff --git a/java/shaded-deps/s3a-aws-sdk/pom.xml b/java/shaded-deps/s3a-aws-sdk/pom.xml
index 26e6995fa..511e86bfe 100644
--- a/java/shaded-deps/s3a-aws-sdk/pom.xml
+++ b/java/shaded-deps/s3a-aws-sdk/pom.xml
@@ -25,7 +25,7 @@ though some of them might not be necessary. The exclusions are sorted alphabetic
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
<relativePath>../../pom.xml</relativePath>
</parent>
<modelVersion>4.0.0</modelVersion>
diff --git a/java/test-corrupt-hive-udfs/pom.xml b/java/test-corrupt-hive-udfs/pom.xml
index d78274ab4..cf6b1c3e3 100644
--- a/java/test-corrupt-hive-udfs/pom.xml
+++ b/java/test-corrupt-hive-udfs/pom.xml
@@ -23,7 +23,7 @@ under the License.
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
diff --git a/java/test-hive-udfs/pom.xml b/java/test-hive-udfs/pom.xml
index 5120c46d7..24ea139e3 100644
--- a/java/test-hive-udfs/pom.xml
+++ b/java/test-hive-udfs/pom.xml
@@ -22,7 +22,7 @@ under the License.
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
diff --git a/java/yarn-extras/pom.xml b/java/yarn-extras/pom.xml
index 2dfb3f051..740b1eaf8 100644
--- a/java/yarn-extras/pom.xml
+++ b/java/yarn-extras/pom.xml
@@ -22,7 +22,7 @@
<parent>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parent</artifactId>
- <version>4.5.0-SNAPSHOT</version>
+ <version>4.5.0-RELEASE</version>
</parent>
<modelVersion>4.0.0</modelVersion>
<artifactId>yarn-extras</artifactId>
(impala) 02/05: IMPALA-13771: Fix heap-use-after-free in Cluster Membership Manager
Posted by pr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
prozsa pushed a commit to branch branch-4.5.0
in repository https://gitbox.apache.org/repos/asf/impala.git
commit fdcf7a1446114785f76876398ac72df9c40c88c8
Author: jasonmfehr <jf...@cloudera.com>
AuthorDate: Wed Feb 19 13:33:54 2025 -0800
IMPALA-13771: Fix heap-use-after-free in Cluster Membership Manager
The ASAN tests identified a heap-use-after-free issue. This patch
fixes that issue by moving the memory release to after its last use.
Local execution of custom-cluster-mgr-test ASAN replicated the same
heap-use-after-free failure.
Testing:
Existing custom-cluster-mgr-test passed locally using debug build.
Existing custom-cluster-mgr-test passed locally using ASAN build.
Change-Id: I4fd2c9faa6daba9274f38238b952c377a07794e9
Reviewed-on: http://gerrit.cloudera.org:8080/22503
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/scheduling/cluster-membership-mgr.cc | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/be/src/scheduling/cluster-membership-mgr.cc b/be/src/scheduling/cluster-membership-mgr.cc
index ab8bf9a61..ce3c211db 100644
--- a/be/src/scheduling/cluster-membership-mgr.cc
+++ b/be/src/scheduling/cluster-membership-mgr.cc
@@ -300,12 +300,15 @@ void ClusterMembershipMgr::UpdateMembership(
RemoveExecutorAndGroup(be_desc, group, new_executor_groups);
}
}
- new_backend_map->erase(item.key);
// If a coordinator is not shutdown gracefully, then it will be deleted here.
if (be_desc.is_coordinator()) {
_removeCoordIfExists(new_state, be_desc);
}
+
+ // Note: be_desc is a reference to item.key, thus this erase must come at the end
+ // of the loop where be_desc is initialized.
+ new_backend_map->erase(item.key);
}
continue;
}
(impala) 01/05: IMPALA-13759: Fix Hive ACID INSERT OVERWRITE base detection
Posted by pr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
prozsa pushed a commit to branch branch-4.5.0
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 72044cbaa71109a1a261b7e95ede8a70afcc0a7a
Author: Csaba Ringhofer <cs...@cloudera.com>
AuthorDate: Wed Feb 19 13:32:56 2025 +0100
IMPALA-13759: Fix Hive ACID INSERT OVERWRITE base detection
Base directory created by INSERT OVERWRITE / TRUNCATE should be
treated differently than bases created by compaction because
IOW/TRUNCATE bases must be accepted even if there is an earlier
open writeId. This scenario can easily occur if there is
a pending write to a single partition, as this doesn't block
an IOW/TRUNCATE to another partition, while the global
minOpenWrite affects whether the base is accepted.
This change updates Impala logic to consider these bases
valid similarly to Hive.
Note that differentiating IOW/TRUNCATE from compaction is
different than in Hive, as metadata files are not considered
in Impala (IMPALA-13769). This can only cause problems when
interacting with earlier Hive versions that did not use
visibilityTxnId in the base path. I don't consider this
to be a significant regression that should block the current
critical fix.
Testing:
- added regression EE/FE tests
Change-Id: I838eaf4f41bae148e558f64288a1370c0908efa4
Reviewed-on: http://gerrit.cloudera.org:8080/22499
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../java/org/apache/impala/util/AcidUtils.java | 20 +++++++++++++++--
.../java/org/apache/impala/util/AcidUtilsTest.java | 18 ++++++++++++++++
tests/query_test/test_acid.py | 25 ++++++++++++++++++++++
3 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/util/AcidUtils.java b/fe/src/main/java/org/apache/impala/util/AcidUtils.java
index c8efe788b..ca767b899 100644
--- a/fe/src/main/java/org/apache/impala/util/AcidUtils.java
+++ b/fe/src/main/java/org/apache/impala/util/AcidUtils.java
@@ -257,9 +257,25 @@ public class AcidUtils {
public boolean check(String dirPath) throws CatalogException {
ParsedBase parsedBase = parseBase(dirPath);
+ // The logic below is not completely in sync with the way Hive works currently:
+ // https://github.com/apache/hive/blob/0759352ddddc793c0e717c460f0e08eb3f14c1e9/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java#L1774-L1797
+ // The main difference is that metadata files are ignored, so Impala cannot
+ // differentiate between bases created by INSERT OVERWRITE / TRUNCATE and compacted
+ // bases created by older Hives (that did not use visibiliyTxnId). See IMPALA-13759
+ // for more info.
if (parsedBase.writeId != SENTINEL_BASE_WRITE_ID) {
- boolean isValid = writeIdList.isValidBase(parsedBase.writeId) &&
- isTxnValid(parsedBase.visibilityTxnId);
+ boolean isValid = false;
+ if (parsedBase.visibilityTxnId != -1) {
+ // Assume that the base is produced by a compaction.
+ isValid = writeIdList.isValidBase(parsedBase.writeId) &&
+ isTxnValid(parsedBase.visibilityTxnId);
+ } else {
+ // Assume that the base is produced by INSERT OVERWRITE or TRUNCATE. It is
+ // possible for isValidBase() to fail in this case, for example if there is
+ // an earlier open writeId in a different partition, so only check whether
+ // the writeId is committed.
+ isValid = writeIdList.isWriteIdValid(parsedBase.writeId);
+ }
if (doStrictCheck && !isValid) {
throw new CatalogException("Invalid base file found " + dirPath);
}
diff --git a/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java b/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java
index 6a17b82d2..04644bf94 100644
--- a/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java
+++ b/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java
@@ -597,6 +597,24 @@ public class AcidUtilsTest {
"delta_0000012_0000012_0000/0000_1"});
}
+ @Test
+ public void testOpenWriteIdBeforeInsertOverwrite() {
+ assertFiltering(new String[]{
+ "base_000001/",
+ "base_000001/0000_0",
+ "delta_000002_000002_0000/",
+ "delta_000002_000002_0000/0000",
+ "base_000003/",
+ "base_000003/0000_0"},
+ // <tbl>:<hwm>:<minOpenWriteId>:<openWriteIds>:<abortedWriteIds>
+ "default.test:3:2:1:",
+ new String[]{
+ // As there is no visibilityTxnId, it is assumed that base_000003 was produced
+ // by an insert overwrite and is accepted even though there is an earlier open
+ // write id (regression test for IMPALA-13759).
+ "base_000003/0000_0"});
+ }
+
@Test
public void testWriteIdListCompare() {
ValidWriteIdList a =
diff --git a/tests/query_test/test_acid.py b/tests/query_test/test_acid.py
index 98931119f..860aee168 100644
--- a/tests/query_test/test_acid.py
+++ b/tests/query_test/test_acid.py
@@ -231,6 +231,12 @@ class TestAcid(ImpalaTestSuite):
commit_req.txnid = txn_id
return self.hive_client.commit_txn(commit_req)
+ def _create_insert_only_acid_table(self, tbl, cols, part_cols=None):
+ part_part = ("partitioned by (%s)" % part_cols) if part_cols else ""
+ self.execute_query("create table {} ({}) {} tblproperties"
+ "('transactional'='true',"
+ "'transactional_properties'='insert_only')".format(tbl, cols, part_part))
+
@SkipIfFS.hive
def test_lock_timings(self, vector, unique_database):
def elapsed_time_for_query(query):
@@ -360,3 +366,22 @@ class TestAcid(ImpalaTestSuite):
# Create a new table and load it in catalogd. Catalogd should not hang.
self.client.execute("CREATE TABLE {0}.tbl (i int)".format(unique_database))
self.client.execute("DESCRIBE {0}.tbl".format(unique_database))
+
+ def test_insert_overwrite_base_detection(self, unique_database):
+ """"Regression test for IMPALA-13759. Checks that the base directory created by
+ INSERT OVERWRITE is detected correctly even if there is an earlier open writeId
+ in another partition."""
+ tbl = unique_database + ".insert_only_table"
+ self._create_insert_only_acid_table(tbl, "i int", "p string")
+ sleep_time_ms = 15000
+ other_partition_insert_handle = self.execute_query_async(
+ 'insert into %s partition (p="a") values (sleep(%d))' % (tbl, sleep_time_ms))
+ self.execute_query('insert into %s partition (p="b") values (2)' % tbl)
+ self.execute_query('insert overwrite %s partition (p="b") values (3)' % tbl)
+ self.execute_query('insert into %s partition (p="b") values (4)' % tbl)
+ result = self.execute_query("select * from %s order by i" % tbl)
+ assert result.data == ['3\tb', '4\tb']
+ self.client.wait_for_finished_timeout(
+ other_partition_insert_handle, 0.001 * sleep_time_ms)
+ result = self.execute_query("select * from %s order by i" % tbl)
+ assert result.data == ['1\ta', '3\tb', '4\tb']