You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by lv...@apache.org on 2019/06/10 16:18:34 UTC
[impala] branch master updated (970a075 -> d753600)
This is an automated email from the ASF dual-hosted git repository.
lv pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.
from 970a075 Revert "Fix docs for catalogd automatic invalidate flags"
new d4c05b9 IMPALA-8638: Fix flaky TestLineage::test_create_table_timestamp
new 1f407fd IMPALA-8635. Use local metastore URIs when checking for Kudu/HMS integration
new f6d40b3 IMPALA-8603: remove confusing logging from custom cluster tests
new 45c6c46 IMPALA-5031: signed overflow is undefined behavior
new d753600 IMPALA-8629: (part 1) Add temp KuduStorageHandler
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/exprs/aggregate-functions-ir.cc | 8 ++++---
be/src/runtime/decimal-value.inline.h | 2 +-
be/src/runtime/multi-precision.h | 2 +-
be/src/util/arithmetic-util.h | 9 ++++++++
be/src/util/runtime-profile-counters.h | 4 +++-
.../java/org/apache/impala/catalog/KuduTable.java | 15 ++++++++-----
.../apache/impala/service/CatalogOpExecutor.java | 9 +-------
.../java/org/apache/impala/util/MetaStoreUtil.java | 11 +++++----
.../apache/impala/analysis/AnalyzeKuduDDLTest.java | 13 ++++++-----
tests/common/impala_connection.py | 2 +-
tests/custom_cluster/test_kudu.py | 10 +++++----
tests/custom_cluster/test_lineage.py | 26 +++++++++-------------
tests/query_test/test_kudu.py | 5 +++--
13 files changed, 64 insertions(+), 52 deletions(-)
[impala] 05/05: IMPALA-8629: (part 1) Add temp KuduStorageHandler
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit d753600f9cf4bda0c955f0f5eecc6fb41b0c365e
Author: Grant Henke <gh...@cloudera.com>
AuthorDate: Fri Jun 7 14:26:51 2019 -0500
IMPALA-8629: (part 1) Add temp KuduStorageHandler
This patch adds a temporary KuduStorageHandler
so that the Kudu project can change its handler without
breaking the integration. It also disabled any tests that
depend on the specific handler.
A follow up patch will remove the
TEMP_KUDU_STORAGE_HANDLER. adjust the
KUDU_STORAGE_HANDLER value to be the final value,
and re-enable the tests.
Change-Id: Ic9982466699818390fa28efc5ea1aae75b11c12a
Reviewed-on: http://gerrit.cloudera.org:8080/13561
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Thomas Marshall <tm...@cloudera.com>
---
fe/src/main/java/org/apache/impala/catalog/KuduTable.java | 9 ++++++++-
.../java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java | 13 +++++++------
tests/custom_cluster/test_kudu.py | 10 ++++++----
tests/query_test/test_kudu.py | 5 +++--
4 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/catalog/KuduTable.java b/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
index 0f0540f..1626236 100644
--- a/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
@@ -87,6 +87,12 @@ public class KuduTable extends Table implements FeKuduTable {
public static final String KUDU_STORAGE_HANDLER =
"org.apache.kudu.hive.KuduStorageHandler";
+ // TODO(IMPALA-8629): Remove this after Kudu adjusts its StorageHandler logic.
+ // Once that is done KUDU_STORAGE_HANDLER will be
+ // "org.apache.hadoop.hive.kudu.KuduStorageHandler".
+ public static final String TEMP_KUDU_STORAGE_HANDLER =
+ "org.apache.hadoop.hive.kudu.KuduStorageHandler";
+
// Key to specify the number of tablet replicas.
public static final String KEY_TABLET_REPLICAS = "kudu.num_tablet_replicas";
@@ -140,7 +146,8 @@ public class KuduTable extends Table implements FeKuduTable {
public static boolean isKuduStorageHandler(String handler) {
return handler != null && (handler.equals(KUDU_LEGACY_STORAGE_HANDLER) ||
- handler.equals(KUDU_STORAGE_HANDLER));
+ handler.equals(KUDU_STORAGE_HANDLER) ||
+ handler.equals(TEMP_KUDU_STORAGE_HANDLER));
}
public static boolean isKuduTable(org.apache.hadoop.hive.metastore.api.Table msTbl) {
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
index 7a6b8e7..20e8f4e 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
@@ -198,13 +198,14 @@ public class AnalyzeKuduDDLTest extends FrontendTestBase {
"partition 30 < values) stored as kudu tblproperties" +
"('kudu.master_addresses' = '%s')", kuduMasters));
// Not using the STORED AS KUDU syntax to specify a Kudu table
- AnalysisError("create table tab (x int) tblproperties (" +
- "'storage_handler'='org.apache.kudu.hive.KuduStorageHandler')",
- CreateTableStmt.KUDU_STORAGE_HANDLER_ERROR_MESSAGE);
+ // TODO(IMPALA-8629): Uncomment tests after Kudu adjusts its StorageHandler logic.
+ // AnalysisError("create table tab (x int) tblproperties (" +
+ // "'storage_handler'='org.apache.kudu.hive.KuduStorageHandler')",
+ // CreateTableStmt.KUDU_STORAGE_HANDLER_ERROR_MESSAGE);
// Creating unpartitioned table results in a warning.
- AnalyzesOk("create table tab (x int primary key) stored as kudu " +
- "tblproperties ('storage_handler'='org.apache.kudu.hive.KuduStorageHandler')",
- "Unpartitioned Kudu tables are inefficient for large data sizes.");
+ // AnalyzesOk("create table tab (x int primary key) stored as kudu " +
+ // "tblproperties ('storage_handler'='org.apache.kudu.hive.KuduStorageHandler')",
+ // "Unpartitioned Kudu tables are inefficient for large data sizes.");
// Invalid value for number of replicas
AnalysisError("create table t (x int primary key) stored as kudu tblproperties (" +
"'kudu.num_tablet_replicas'='1.1')",
diff --git a/tests/custom_cluster/test_kudu.py b/tests/custom_cluster/test_kudu.py
index e3d6cc0..eb87a1e 100644
--- a/tests/custom_cluster/test_kudu.py
+++ b/tests/custom_cluster/test_kudu.py
@@ -168,8 +168,9 @@ class TestKuduHMSIntegration(CustomClusterTestSuite, KuduTestSuite):
assert ["", "EXTERNAL", "TRUE"] in table_desc
assert ["", "kudu.master_addresses", KUDU_MASTER_HOSTS] in table_desc
assert ["", "kudu.table_name", kudu_table.name] in table_desc
- assert ["", "storage_handler", "org.apache.kudu.hive.KuduStorageHandler"] \
- in table_desc
+ # TODO(IMPALA-8629): Uncomment tests after Kudu adjusts its StorageHandler logic.
+ # assert ["", "storage_handler", "org.apache.kudu.hive.KuduStorageHandler"] \
+ # in table_desc
@pytest.mark.execute_serially
def test_implicit_managed_table_props(self, cursor, kudu_client, unique_database):
@@ -190,8 +191,9 @@ class TestKuduHMSIntegration(CustomClusterTestSuite, KuduTestSuite):
assert any("kudu.master_addresses" in s for s in table_desc)
assert ["Table Type:", "MANAGED_TABLE", None] in table_desc
assert ["", "kudu.table_name", "%s.foo" % unique_database] in table_desc
- assert ["", "storage_handler", "org.apache.kudu.hive.KuduStorageHandler"] \
- in table_desc
+ # TODO(IMPALA-8629): Uncomment tests after Kudu adjusts its StorageHandler logic.
+ # assert ["", "storage_handler", "org.apache.kudu.hive.KuduStorageHandler"] \
+ # in table_desc
@pytest.mark.execute_serially
def test_drop_non_empty_db(self, unique_cursor, kudu_client):
diff --git a/tests/query_test/test_kudu.py b/tests/query_test/test_kudu.py
index 1dc5eba..875b5d0 100644
--- a/tests/query_test/test_kudu.py
+++ b/tests/query_test/test_kudu.py
@@ -560,8 +560,9 @@ class TestCreateExternalTable(KuduTestSuite):
assert ["", "EXTERNAL", "TRUE"] in table_desc
assert ["", "kudu.master_addresses", KUDU_MASTER_HOSTS] in table_desc
assert ["", "kudu.table_name", kudu_table.name] in table_desc
- assert ["", "storage_handler", "org.apache.kudu.hive.KuduStorageHandler"] \
- in table_desc
+ # TODO(IMPALA-8629): Uncomment tests after Kudu adjusts its StorageHandler logic.
+ # assert ["", "storage_handler", "org.apache.kudu.hive.KuduStorageHandler"] \
+ # in table_desc
@SkipIfKudu.hms_integration_enabled
def test_col_types(self, cursor, kudu_client):
[impala] 01/05: IMPALA-8638: Fix flaky
TestLineage::test_create_table_timestamp
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit d4c05b96e2f1166572b01cae31b20ecd22fa4094
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Fri Jun 7 10:29:55 2019 -0700
IMPALA-8638: Fix flaky TestLineage::test_create_table_timestamp
This patch fixes the bug TestLineage::test_create_table_timestamp where
it uses the same lineage log directory created by
TestLineage::test_start_end_timestamp, which could fail the query ID
assertion. The fix is to use a different lineage log directory than the
one used by TestLineage::test_start_end_timestamp.
Testing:
- Ran test_lineage.py multiple times and it still passed.
Change-Id: I5813e4a570c181ba196b9ddf0210c8a0d92e21e8
Reviewed-on: http://gerrit.cloudera.org:8080/13560
Reviewed-by: Fredy Wijaya <fw...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
tests/custom_cluster/test_lineage.py | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/tests/custom_cluster/test_lineage.py b/tests/custom_cluster/test_lineage.py
index 35469aa..62746cb 100644
--- a/tests/custom_cluster/test_lineage.py
+++ b/tests/custom_cluster/test_lineage.py
@@ -16,15 +16,12 @@
# under the License.
#
# Tests for column lineage.
-# TODO: add verification for more fields.
import json
import logging
import os
import pytest
import re
-import shutil
-import stat
import tempfile
import time
@@ -34,22 +31,20 @@ LOG = logging.getLogger(__name__)
class TestLineage(CustomClusterTestSuite):
- lineage_log_dir = tempfile.mkdtemp()
+ START_END_TIME_LINEAGE_LOG_DIR = tempfile.mkdtemp(prefix="start_end_time")
+ CREATE_TABLE_TIME_LINEAGE_LOG_DIR = tempfile.mkdtemp(prefix="create_table_time")
@classmethod
def setup_class(cls):
super(TestLineage, cls).setup_class()
- @classmethod
- def teardown_class(cls):
- shutil.rmtree(cls.lineage_log_dir)
-
@pytest.mark.execute_serially
- @CustomClusterTestSuite.with_args("--lineage_event_log_dir=%s" % lineage_log_dir)
+ @CustomClusterTestSuite.with_args("--lineage_event_log_dir={0}"
+ .format(START_END_TIME_LINEAGE_LOG_DIR))
def test_start_end_timestamp(self, vector):
"""Test that 'timestamp' and 'endTime' in the lineage graph are populated with valid
UNIX times."""
- LOG.info("lineage_event_log_dir is " + self.lineage_log_dir)
+ LOG.info("lineage_event_log_dir is {0}".format(self.START_END_TIME_LINEAGE_LOG_DIR))
before_time = int(time.time())
query = "select count(*) from functional.alltypes"
result = self.execute_query_expect_success(self.client, query)
@@ -60,8 +55,8 @@ class TestLineage(CustomClusterTestSuite):
# Stop the cluster in order to flush the lineage log files.
self._stop_impala_cluster()
- for log_filename in os.listdir(self.lineage_log_dir):
- log_path = os.path.join(self.lineage_log_dir, log_filename)
+ for log_filename in os.listdir(self.START_END_TIME_LINEAGE_LOG_DIR):
+ log_path = os.path.join(self.START_END_TIME_LINEAGE_LOG_DIR, log_filename)
# Only the coordinator's log file will be populated.
if os.path.getsize(log_path) > 0:
LOG.info("examining file: " + log_path)
@@ -77,7 +72,8 @@ class TestLineage(CustomClusterTestSuite):
LOG.info("empty file: " + log_path)
@pytest.mark.execute_serially
- @CustomClusterTestSuite.with_args("--lineage_event_log_dir={0}".format(lineage_log_dir))
+ @CustomClusterTestSuite.with_args("--lineage_event_log_dir={0}"
+ .format(CREATE_TABLE_TIME_LINEAGE_LOG_DIR))
def test_create_table_timestamp(self, vector, unique_database):
"""Test that 'createTableTime' in the lineage graph are populated with valid value
from HMS."""
@@ -89,8 +85,8 @@ class TestLineage(CustomClusterTestSuite):
# Wait to flush the lineage log files.
time.sleep(3)
- for log_filename in os.listdir(self.lineage_log_dir):
- log_path = os.path.join(self.lineage_log_dir, log_filename)
+ for log_filename in os.listdir(self.CREATE_TABLE_TIME_LINEAGE_LOG_DIR):
+ log_path = os.path.join(self.CREATE_TABLE_TIME_LINEAGE_LOG_DIR, log_filename)
# Only the coordinator's log file will be populated.
if os.path.getsize(log_path) > 0:
with open(log_path) as log_file:
[impala] 03/05: IMPALA-8603: remove confusing logging from custom
cluster tests
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit f6d40b37c8b13131cbc75db6a7327c34bf0c5a34
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
AuthorDate: Mon Jun 3 15:12:37 2019 -0700
IMPALA-8603: remove confusing logging from custom cluster tests
IMPALA-1653 added explicit closing of the HS2 session that is created
by ImpalaTestSuite during teardown. For the custom cluster tests, this
sometimes fails, either because the impalad was shutdown and is no
longer reachable or because it was restarted and the new impalad
doesn't know about the session that was created before restart.
Currently, when these errors occur we log them. However, this is
confusing because several custom cluster tests now log errors even
though they succeed. This patch changes this to simply ignore the
errors.
One concern is that a test may hit a genuine error here and if so we
won't have the logging for it. If this happens, though, it should
generally be possible for a dev to repro the error with the logging
enabled.
Change-Id: I96144fdbe6cc9bf0f01a9951420ee6f281fa6649
Reviewed-on: http://gerrit.cloudera.org:8080/13502
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
tests/common/impala_connection.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/common/impala_connection.py b/tests/common/impala_connection.py
index b05478f..5242090 100644
--- a/tests/common/impala_connection.py
+++ b/tests/common/impala_connection.py
@@ -283,7 +283,7 @@ class ImpylaHS2Connection(ImpalaConnection):
self.__cursor.close()
except Exception, e:
# The session may no longer be valid if the impalad was restarted during the test.
- LOG.exception(e)
+ pass
self.__impyla_conn.close()
def close_query(self, operation_handle):
[impala] 04/05: IMPALA-5031: signed overflow is undefined behavior
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 45c6c46bf6d051c715fe75a68c7970b208a7d376
Author: Jim Apple <jb...@apache.org>
AuthorDate: Sat May 25 17:50:21 2019 -0700
IMPALA-5031: signed overflow is undefined behavior
Fix remaining signed overflow undefined behaviors in end-to-end
tests. The interesting part of the backtraces:
exprs/aggregate-functions-ir.cc:464:25: runtime error: signed
integer overflow: 0x5a4728ca063b522c0b728f8000000000 +
0x3c2f7086aed236c807a1b50000000000 cannot be represented in
type '__int128'
#0 AggregateFunctions::DecimalAvgMerge(
impala_udf::FunctionContext*, impala_udf::StringVal const&,
impala_udf::StringVal*) exprs/aggregate-functions-ir.cc:464:25
#1 AggFnEvaluator::Update(TupleRow const*, Tuple*, void*)
exprs/agg-fn-evaluator.cc:327:7
#2 AggFnEvaluator::Add(TupleRow const*, Tuple*)
exprs/agg-fn-evaluator.h:257:3
#3 Aggregator::UpdateTuple(AggFnEvaluator**, Tuple*, TupleRow*, bool)
exec/aggregator.cc:167:24
#4 NonGroupingAggregator::AddBatchImpl(RowBatch*)
exec/non-grouping-aggregator-ir.cc:27:5
#5 NonGroupingAggregator::AddBatch(RuntimeState*, RowBatch*)
exec/non-grouping-aggregator.cc:124:45
#6 AggregationNode::Open(RuntimeState*)
exec/aggregation-node.cc:70:57
exprs/aggregate-functions-ir.cc:513:12: runtime error: signed
integer overflow: -8282081183197145958 + -4473782455107795527
cannot be represented in type 'long'
#0 void AggregateFunctions::SumUpdate<impala_udf::BigIntVal,
impala_udf::BigIntVal>(impala_udf::FunctionContext*,
impala_udf::BigIntVal const&, impala_udf::BigIntVal*)
exprs/aggregate-functions-ir.cc:513:12
#1 AggFnEvaluator::Update(TupleRow const*, Tuple*, void*)
exprs/agg-fn-evaluator.cc:327:7
#2 AggFnEvaluator::Add(TupleRow const*, Tuple*)
exprs/agg-fn-evaluator.h:257:3
#3 Aggregator::UpdateTuple(AggFnEvaluator**, Tuple*, TupleRow*,
bool) exec/aggregator.cc:167:24
#4 NonGroupingAggregator::AddBatchImpl(RowBatch*)
exec/non-grouping-aggregator-ir.cc:27:5
#5 NonGroupingAggregator::AddBatch(RuntimeState*, RowBatch*)
exec/non-grouping-aggregator.cc:124:45
#6 AggregationNode::Open(RuntimeState*)
exec/aggregation-node.cc:70:57
exprs/aggregate-functions-ir.cc:585:14: runtime error: signed
integer overflow: 0x5a4728ca063b522c0b728f8000000000 +
0x3c2f7086aed236c807a1b50000000000 cannot be represented in
type '__int128'
#0 AggregateFunctions::SumDecimalMerge(
impala_udf::FunctionContext*, impala_udf::DecimalVal const&,
impala_udf::DecimalVal*) exprs/aggregate-functions-ir.cc:585:14
#1 AggFnEvaluator::Update(TupleRow const*, Tuple*, void*)
exprs/agg-fn-evaluator.cc:327:7
#2 AggFnEvaluator::Add(TupleRow const*, Tuple*)
exprs/agg-fn-evaluator.h:257:3
#3 Aggregator::UpdateTuple(AggFnEvaluator**, Tuple*, TupleRow*, bool)
exec/aggregator.cc:167:24
#4 NonGroupingAggregator::AddBatchImpl(RowBatch*)
exec/non-grouping-aggregator-ir.cc:27:5
#5 NonGroupingAggregator::AddBatch(RuntimeState*, RowBatch*)
exec/non-grouping-aggregator.cc:124:45
#6 AggregationNode::Open(RuntimeState*)
exec/aggregation-node.cc:70:57
runtime/decimal-value.inline.h:145:12: runtime error: signed
integer overflow: 18 * 0x0785ee10d5da46d900f436a000000000 cannot
be represented in type '__int128'
#0 DecimalValue<__int128>::ScaleTo(int, int, int, bool*) const
runtime/decimal-value.inline.h:145:12
#1 DecimalOperators::ScaleDecimalValue(
impala_udf::FunctionContext*, DecimalValue<int> const&, int,
int, int) exprs/decimal-operators-ir.cc:132:41
#2 DecimalOperators::RoundDecimal(impala_udf::FunctionContext*,
impala_udf::DecimalVal const&, int, int, int, int,
DecimalOperators::DecimalRoundOp const&)
exprs/decimal-operators-ir.cc:465:16
#3 DecimalOperators::RoundDecimal(impala_udf::FunctionContext*,
impala_udf::DecimalVal const&, DecimalOperators::DecimalRoundOp
const&) exprs/decimal-operators-ir.cc:519:10
#4 DecimalOperators::CastToDecimalVal(
impala_udf::FunctionContext*, impala_udf::DecimalVal const&)
exprs/decimal-operators-ir.cc:529:10
#5 impala_udf::DecimalVal ScalarFnCall::InterpretEval
<impala_udf::DecimalVal>(ScalarExprEvaluator*, TupleRow const*)
const exprs/scalar-fn-call.cc:485:208
#6 ScalarFnCall::GetDecimalVal(ScalarExprEvaluator*, TupleRow
const*) const exprs/scalar-fn-call.cc:618:44
#7 ScalarExprEvaluator::GetValue(ScalarExpr const&, TupleRow
const*) exprs/scalar-expr-evaluator.cc:321:27
#8 ScalarExprEvaluator::GetValue(TupleRow const*)
exprs/scalar-expr-evaluator.cc:251:10
#9 Java_org_apache_impala_service_FeSupport_NativeEvalExprsWithoutRow
service/fe-support.cc:246:26
#10 (<unknown module>)
runtime/multi-precision.h:116:21: runtime error: negation of
0x80000000000000000000000000000000 cannot be represented in
type 'int128_t' (aka '__int128'); cast to an unsigned type to
negate this value to itself
#0 ConvertToInt128(boost::multiprecision::number
<boost::multiprecision::backends::cpp_int_backend<256u, 256u,
(boost::multiprecision::cpp_integer_type)1,
(boost::multiprecision::cpp_int_check_type)0, void>,
(boost::multiprecision::expression_template_option)0>,
__int128, bool*) runtime/multi-precision.h:116:21
#1 DecimalValue<__int128>
DecimalValue<__int128>::Multiply<__int128>(int,
DecimalValue<__int128> const&, int, int, int, bool, bool*) const
runtime/decimal-value.inline.h:438:16
#2 DecimalOperators::Multiply_DecimalVal_DecimalVal(
impala_udf::FunctionContext*, impala_udf::DecimalVal const&,
impala_udf::DecimalVal const&)
exprs/decimal-operators-ir.cc:859:3336
#3 impala_udf::DecimalVal ScalarFnCall::InterpretEval
<impala_udf::DecimalVal>(ScalarExprEvaluator*, TupleRow const*)
const exprs/scalar-fn-call.cc:485:376
#4 ScalarFnCall::GetDecimalVal(ScalarExprEvaluator*, TupleRow
const*) const exprs/scalar-fn-call.cc:618:44
#5 ScalarExprEvaluator::GetValue(ScalarExpr const&, TupleRow
const*) exprs/scalar-expr-evaluator.cc:321:27
#6 ScalarExprEvaluator::GetValue(TupleRow const*)
exprs/scalar-expr-evaluator.cc:251:10
#7 Java_org_apache_impala_service_FeSupport_NativeEvalExprsWithoutRow
service/fe-support.cc:246:26
#8 (<unknown module>)
util/runtime-profile-counters.h:194:24: runtime error: signed
integer overflow: -1263418397011577524 + -9223370798768111350
cannot be represented in type 'long'
#0 RuntimeProfile::AveragedCounter::UpdateCounter
(RuntimeProfile::Counter*)
util/runtime-profile-counters.h:194:24
#1 RuntimeProfile::UpdateAverage(RuntimeProfile*)
util/runtime-profile.cc:199:20
#2 RuntimeProfile::UpdateAverage(RuntimeProfile*)
util/runtime-profile.cc:245:14
#3 Coordinator::BackendState::UpdateExecStats
(vector<Coordinator::FragmentStats*,
allocator<Coordinator::FragmentStats*> > const&)
runtime/coordinator-backend-state.cc:429:22
#4 Coordinator::ComputeQuerySummary()
runtime/coordinator.cc:775:20
#5 Coordinator::HandleExecStateTransition(Coordinator::ExecState,
Coordinator::ExecState) runtime/coordinator.cc:567:3
#6 Coordinator::SetNonErrorTerminalState(Coordinator::ExecState)
runtime/coordinator.cc:484:3
#7 Coordinator::GetNext(QueryResultSet*, int, bool*)
runtime/coordinator.cc:657:53
#8 ClientRequestState::FetchRowsInternal(int, QueryResultSet*)
service/client-request-state.cc:943:34
#9 ClientRequestState::FetchRows(int, QueryResultSet*)
service/client-request-state.cc:835:36
#10 ImpalaServer::FetchInternal(TUniqueId const&, bool, int,
beeswax::Results*) service/impala-beeswax-server.cc:545:40
#11 ImpalaServer::fetch(beeswax::Results&, beeswax::QueryHandle
const&, bool, int) service/impala-beeswax-server.cc:178:19
#12 beeswax::BeeswaxServiceProcessor::process_fetch(int,
apache::thrift::protocol::TProtocol*,
apache::thrift::protocol::TProtocol*, void*)
generated-sources/gen-cpp/BeeswaxService.cpp:3398:13
#13 beeswax::BeeswaxServiceProcessor::dispatchCall
(apache::thrift::protocol::TProtocol*,
apache::thrift::protocol::TProtocol*, string const&, int,
void*) generated-sources/gen-cpp/BeeswaxService.cpp:3200:3
#14 ImpalaServiceProcessor::dispatchCall
(apache::thrift::protocol::TProtocol*,
apache::thrift::protocol::TProtocol*, string const&, int,
void*) generated-sources/gen-cpp/ImpalaService.cpp:1824:48
#15 apache::thrift::TDispatchProcessor::process
(boost::shared_ptr<apache::thrift::protocol::TProtocol>,
boost::shared_ptr<apache::thrift::protocol::TProtocol>, void*)
toolchain/thrift-0.9.3-p5/include/thrift/TDispatchProcessor.h:121:12
Change-Id: I73dd6802ec1023275d09a99a2950f3558313fc8e
Reviewed-on: http://gerrit.cloudera.org:8080/13437
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/exprs/aggregate-functions-ir.cc | 8 +++++---
be/src/runtime/decimal-value.inline.h | 2 +-
be/src/runtime/multi-precision.h | 2 +-
be/src/util/arithmetic-util.h | 9 +++++++++
be/src/util/runtime-profile-counters.h | 4 +++-
5 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/be/src/exprs/aggregate-functions-ir.cc b/be/src/exprs/aggregate-functions-ir.cc
index e584cca..1c9bd85 100644
--- a/be/src/exprs/aggregate-functions-ir.cc
+++ b/be/src/exprs/aggregate-functions-ir.cc
@@ -36,6 +36,7 @@
#include "runtime/string-value.inline.h"
#include "runtime/timestamp-value.h"
#include "runtime/timestamp-value.inline.h"
+#include "util/arithmetic-util.h"
#include "util/mpfit-util.h"
#include "common/names.h"
@@ -461,7 +462,8 @@ void AggregateFunctions::DecimalAvgMerge(FunctionContext* ctx,
abs(dst_struct->sum_val16) >
DecimalUtil::MAX_UNSCALED_DECIMAL16 - abs(src_struct->sum_val16);
if (UNLIKELY(overflow)) ctx->SetError("Avg computation overflowed");
- dst_struct->sum_val16 += src_struct->sum_val16;
+ dst_struct->sum_val16 =
+ ArithmeticUtil::AsUnsigned<std::plus>(dst_struct->sum_val16, src_struct->sum_val16);
dst_struct->count += src_struct->count;
}
@@ -510,7 +512,7 @@ void AggregateFunctions::SumUpdate(FunctionContext* ctx, const SRC_VAL& src,
return;
}
if (dst->is_null) InitZero<DST_VAL>(ctx, dst);
- dst->val += src.val;
+ dst->val = ArithmeticUtil::Compute<std::plus, decltype(dst->val)>(dst->val, src.val);
}
template<typename SRC_VAL, typename DST_VAL>
@@ -582,7 +584,7 @@ void AggregateFunctions::SumDecimalMerge(FunctionContext* ctx,
bool overflow = decimal_v2 &&
abs(dst->val16) > DecimalUtil::MAX_UNSCALED_DECIMAL16 - abs(src.val16);
if (UNLIKELY(overflow)) ctx->SetError("Sum computation overflowed");
- dst->val16 += src.val16;
+ dst->val16 = ArithmeticUtil::AsUnsigned<std::plus>(dst->val16, src.val16);
}
template<typename T>
diff --git a/be/src/runtime/decimal-value.inline.h b/be/src/runtime/decimal-value.inline.h
index 0a4866e..6480099 100644
--- a/be/src/runtime/decimal-value.inline.h
+++ b/be/src/runtime/decimal-value.inline.h
@@ -142,7 +142,7 @@ inline DecimalValue<T> DecimalValue<T>::ScaleTo(int src_scale, int dst_scale,
} else if (delta_scale < 0) {
T mult = DecimalUtil::GetScaleMultiplier<T>(-delta_scale);
*overflow |= abs(result) >= max_value / mult;
- result *= mult;
+ result = ArithmeticUtil::AsUnsigned<std::multiplies>(result, mult);
}
return DecimalValue(result);
}
diff --git a/be/src/runtime/multi-precision.h b/be/src/runtime/multi-precision.h
index 0d8028f..5abc6a3 100644
--- a/be/src/runtime/multi-precision.h
+++ b/be/src/runtime/multi-precision.h
@@ -113,7 +113,7 @@ inline int128_t ConvertToInt128(int256_t x, int128_t max_value, bool* overflow)
scale =
ArithmeticUtil::AsUnsigned<std::multiplies>(scale, static_cast<int128_t>(base));
}
- return negative ? -result : result;
+ return negative ? ArithmeticUtil::Negate(result) : result;
}
/// abs() is not defined for int128_t. Name it abs() so it can be compatible with
diff --git a/be/src/util/arithmetic-util.h b/be/src/util/arithmetic-util.h
index 4ab60cd..d6a6446 100644
--- a/be/src/util/arithmetic-util.h
+++ b/be/src/util/arithmetic-util.h
@@ -113,6 +113,15 @@ class ArithmeticUtil {
return OperateOn<T>::template Compute<Operator>(x, y);
}
+ // Negation of the least value of signed two's-complement types is undefined behavior.
+ // This operator makes that behavior defined by doing it in the unsigned domain. Note
+ // that this induces Negate(INT_MIN) == INT_MIN, though otherwise produces identical
+ // behavior to just using the usual unary negation operator like "-x".
+ template<typename T>
+ static T Negate(T x) {
+ return ToSigned(-ToUnsigned(x));
+ }
+
private:
// Ring and OperateOn are used for compile-time dispatching on how Compute() should
// perform an arithmetic operation: as an unsigned integer operation, as a
diff --git a/be/src/util/runtime-profile-counters.h b/be/src/util/runtime-profile-counters.h
index a9b3875..cb557ce 100644
--- a/be/src/util/runtime-profile-counters.h
+++ b/be/src/util/runtime-profile-counters.h
@@ -26,6 +26,7 @@
#include "common/atomic.h"
#include "common/logging.h"
+#include "util/arithmetic-util.h"
#include "util/runtime-profile.h"
#include "util/stopwatch.h"
#include "util/streaming-sampler.h"
@@ -191,7 +192,8 @@ class RuntimeProfile::AveragedCounter : public RuntimeProfile::Counter {
double result_val = current_double_sum_ / (double) counter_value_map_.size();
value_.Store(*reinterpret_cast<int64_t*>(&result_val));
} else {
- current_int_sum_ += (new_counter->value() - old_val);
+ current_int_sum_ = ArithmeticUtil::AsUnsigned<std::plus>(
+ current_int_sum_, (new_counter->value() - old_val));
value_.Store(current_int_sum_ / counter_value_map_.size());
}
}
[impala] 02/05: IMPALA-8635. Use local metastore URIs when checking
for Kudu/HMS integration
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 1f407fd8d347618fdc18a85b5350556681998ce3
Author: Todd Lipcon <to...@apache.org>
AuthorDate: Thu Jun 6 23:41:17 2019 -0700
IMPALA-8635. Use local metastore URIs when checking for Kudu/HMS integration
The patch for IMPALA-8504 (part 2) (6bb404dc359) checks to see if Impala
and Kudu are configured against the same metastore to determine if the
HMS integration is enabled. However, instead of using its own metastore
URI config, it uses the configuration stored on the remote HMS. This is
error prone because it's not common for the HMS configuration to store
its own URI. Instead, we should use our own config.
This patch changes to using the local configuration for this purpose.
More robust would be to use the HMS "UUID" support, since it's possible
that Kudu and Impala are talking to different HMS instances sharing a
backing DB, but that work is deferred to a later commit since it depends
on Kudu-side changes.
This commit doesn't add any tests, but fixes the existing tests when
running against Hive 3, where the HMS server side uses a different
configuration key for the metastore URIs.
Change-Id: Id7a4c2cc0580f7c4dc5cfceed30b91e87c547612
Reviewed-on: http://gerrit.cloudera.org:8080/13555
Reviewed-by: Hao Hao <ha...@cloudera.com>
Reviewed-by: Thomas Marshall <tm...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
fe/src/main/java/org/apache/impala/catalog/KuduTable.java | 6 +-----
.../java/org/apache/impala/service/CatalogOpExecutor.java | 9 +--------
fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java | 11 +++++++----
3 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/catalog/KuduTable.java b/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
index e059013..0f0540f 100644
--- a/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
@@ -212,11 +212,7 @@ public class KuduTable extends Table implements FeKuduTable {
public static boolean isHMSIntegrationEnabledAndValidate(String kuduMasters,
String hmsUris) throws ImpalaRuntimeException {
Preconditions.checkNotNull(hmsUris);
- // Skip validation if the HMS URIs in impala is empty for some reason.
- // TODO: Is this a valid case?
- if (hmsUris.isEmpty()) {
- return true;
- }
+ Preconditions.checkArgument(!hmsUris.isEmpty());
HiveMetastoreConfig hmsConfig = getHiveMetastoreConfig(kuduMasters);
if (hmsConfig == null) {
return false;
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index c16bc38..63d4166 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -1515,14 +1515,7 @@ public class CatalogOpExecutor {
// the configuration.
Preconditions.checkState(KuduTable.isKuduTable(msTbl));
String masterHosts = msTbl.getParameters().get(KuduTable.KEY_MASTER_HOSTS);
- String hmsUris;
- try {
- hmsUris = MetaStoreUtil.getHiveMetastoreUrisKeyValue(
- catalog_.getMetaStoreClient().getHiveClient());
- } catch (Exception e) {
- throw new RuntimeException(String.format("Failed to get the Hive Metastore " +
- "configuration for table '%s' ", msTbl.getTableName()), e);
- }
+ String hmsUris = MetaStoreUtil.getHiveMetastoreUris();
return KuduTable.isHMSIntegrationEnabledAndValidate(masterHosts, hmsUris);
}
diff --git a/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java b/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
index 789892d..ee026d8 100644
--- a/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
+++ b/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
@@ -88,6 +88,8 @@ public class MetaStoreUtil {
// The default value for the above configuration key.
public static final String DEFAULT_HIVE_METASTORE_URIS = "";
+ public static final String hiveMetastoreUris_;
+
static {
// Get the value from the Hive configuration, if present.
HiveConf hiveConf = new HiveConf(HdfsTable.class);
@@ -105,6 +107,9 @@ public class MetaStoreUtil {
"default: %d", maxPartitionsPerRpc_, DEFAULT_MAX_PARTITIONS_PER_RPC));
maxPartitionsPerRpc_ = DEFAULT_MAX_PARTITIONS_PER_RPC;
}
+
+ hiveMetastoreUris_ = hiveConf.get(HIVE_METASTORE_URIS_KEY,
+ DEFAULT_HIVE_METASTORE_URIS);
}
/**
@@ -119,10 +124,8 @@ public class MetaStoreUtil {
/**
* Return the value of thrift URI for the remote Hive Metastore.
*/
- public static String getHiveMetastoreUrisKeyValue(IMetaStoreClient client)
- throws ConfigValSecurityException, TException {
- return client.getConfigValue(
- HIVE_METASTORE_URIS_KEY, DEFAULT_HIVE_METASTORE_URIS);
+ public static String getHiveMetastoreUris() {
+ return hiveMetastoreUris_;
}
/**