You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/05/12 18:34:16 UTC
[impala] branch master updated (832c9de -> e4352aa)
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.
from 832c9de IMPALA-4658: Potential race if compiler reorders ReachedLimit() usage.
new 6140c1c IMPALA-8281: Misc Sentry decoupling clean up
new e2ead7f expr-test: use gtest parameterization
new e4352aa IMPALA-7665: Fix unwarranted query cancellation on statestore restart
The 3 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/expr-test.cc | 260 +++++++++++----------
be/src/service/impala-server.cc | 13 +-
be/src/statestore/statestore-subscriber.cc | 5 +-
be/src/statestore/statestore-subscriber.h | 7 +
common/thrift/Frontend.thrift | 4 +-
.../apache/impala/analysis/CreateDropRoleStmt.java | 6 -
.../impala/analysis/GrantRevokePrivStmt.java | 14 +-
.../impala/analysis/GrantRevokeRoleStmt.java | 3 -
.../impala/analysis/ShowGrantPrincipalStmt.java | 24 --
.../org/apache/impala/analysis/ShowRolesStmt.java | 1 -
.../sentry/SentryCatalogdAuthorizationManager.java | 62 ++++-
.../sentry/SentryImpaladAuthorizationManager.java | 44 +++-
.../java/org/apache/impala/service/Frontend.java | 31 +--
.../impala/analysis/AnalyzeAuthStmtsTest.java | 13 --
tests/authorization/test_sentry.py | 42 ++++
tests/custom_cluster/test_restart_services.py | 68 ++++++
16 files changed, 366 insertions(+), 231 deletions(-)
[impala] 02/03: expr-test: use gtest parameterization
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit e2ead7f8572c6005f23f8654305ea8eb389c7c5a
Author: Todd Lipcon <to...@apache.org>
AuthorDate: Wed May 8 16:14:01 2019 -0700
expr-test: use gtest parameterization
Instead of running the tests three times with different flags from
main(), this uses gtest's parameterization feature to accomplish the
same.
The advantage here is that we end up with different test names for each
of the runs. Additionally, this moves the setup code into a proper setup
method so that executing expr-test --gtest_list_tests doesn't waste time
starting a cluster.
This is prep work towards adding multi-threaded test execution for
long-running tests. expr-test seems to currently be one of the worst
offenders.
Change-Id: Idc9fb24ad62b4aa2e120a99d74ae04bb221c034b
Reviewed-on: http://gerrit.cloudera.org:8080/13289
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/exprs/expr-test.cc | 260 ++++++++++++++++++++++++----------------------
1 file changed, 135 insertions(+), 125 deletions(-)
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index dbdb8a1..4e32ce5 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -94,8 +94,6 @@ const char* TEST_TZ_WITHOUT_DST = "America/Anguilla";
ImpaladQueryExecutor* executor_;
scoped_ptr<MetricGroup> statestore_metrics(new MetricGroup("statestore_metrics"));
Statestore* statestore;
-bool disable_codegen_;
-bool enable_expr_rewrites_;
template <typename ORIGINAL_TYPE, typename VAL_TYPE>
string LiteralToString(VAL_TYPE val) {
@@ -189,7 +187,50 @@ class ScopedLocalUnixTimestampConversionOverride {
}
};
-class ExprTest : public testing::Test {
+class ExprTest : public testing::TestWithParam<std::tuple<bool, bool>> {
+ public:
+ // Run once (independent of parameter values).
+ static void SetUpTestCase() {
+ InitFeSupport(false);
+ ABORT_IF_ERROR(impala::LlvmCodeGen::InitializeLlvm());
+
+ // The host running this test might have an out-of-date tzdata package installed.
+ // To avoid tzdata related issues, we will load time-zone db from the testdata
+ // directory.
+ FLAGS_hdfs_zone_info_zip = Substitute("file://$0/testdata/tzdb/2017c.zip",
+ getenv("IMPALA_HOME"));
+ ABORT_IF_ERROR(TimezoneDatabase::Initialize());
+
+ // Disable llvm optimization passes if the env var is no set to true. Running without
+ // the optimizations makes the tests run much faster.
+ char* optimizations = getenv("EXPR_TEST_ENABLE_OPTIMIZATIONS");
+ if (optimizations != NULL && strcmp(optimizations, "true") == 0) {
+ cout << "Running with optimization passes." << endl;
+ FLAGS_disable_optimization_passes = false;
+ } else {
+ cout << "Running without optimization passes." << endl;
+ FLAGS_disable_optimization_passes = true;
+ }
+
+ // Create an in-process Impala server and in-process backends for test environment
+ // without any startup validation check
+ FLAGS_abort_on_config_error = false;
+ VLOG_CONNECTION << "creating test env";
+ VLOG_CONNECTION << "starting backends";
+ statestore = new Statestore(statestore_metrics.get());
+ IGNORE_LEAKING_OBJECT(statestore);
+
+ // Pass in 0 to have the statestore use an ephemeral port for the service.
+ ABORT_IF_ERROR(statestore->Init(0));
+ InProcessImpalaServer* impala_server;
+ ABORT_IF_ERROR(InProcessImpalaServer::StartWithEphemeralPorts(
+ FLAGS_hostname, statestore->port(), &impala_server));
+ IGNORE_LEAKING_OBJECT(impala_server);
+
+ executor_ = new ImpaladQueryExecutor(FLAGS_hostname, impala_server->GetBeeswaxPort());
+ ABORT_IF_ERROR(executor_->Setup());
+ }
+
protected:
// Pool for objects to be destroyed during test teardown.
ObjectPool pool_;
@@ -222,7 +263,27 @@ class ExprTest : public testing::Test {
TimestampValue default_timestamp_val_;
DateValue default_date_val_;
+ bool disable_codegen_;
+ bool enable_expr_rewrites_;
+
virtual void SetUp() {
+ disable_codegen_ = std::get<0>(GetParam());
+ enable_expr_rewrites_ = std::get<1>(GetParam());
+ LOG(INFO) << Substitute(
+ "Test case: disable_codegen=$0 enable_expr_rewrites=$1",
+ disable_codegen_, enable_expr_rewrites_);
+ executor_->ClearExecOptions();
+ executor_->PushExecOption(Substitute("DISABLE_CODEGEN=$0",
+ disable_codegen_ ? 1 : 0));
+ executor_->PushExecOption(Substitute("ENABLE_EXPR_REWRITES=$0",
+ enable_expr_rewrites_ ? 1 : 0));
+
+ // The following have no effect when codegen is disabled, but don't
+ // harm anything either. They generally prevent the planner from doing
+ // anything clever here.
+ executor_->PushExecOption("EXEC_SINGLE_NODE_ROWS_THRESHOLD=0");
+ executor_->PushExecOption("DISABLE_CODEGEN_ROWS_THRESHOLD=0");
+
min_int_values_[TYPE_TINYINT] = 1;
min_int_values_[TYPE_SMALLINT] =
static_cast<int64_t>(numeric_limits<int8_t>::max()) + 1;
@@ -1333,7 +1394,7 @@ void ExprTest::TestSingleLiteralConstruction(
state.ReleaseResources();
}
-TEST_F(ExprTest, NullLiteral) {
+TEST_P(ExprTest, NullLiteral) {
for (int type = TYPE_BOOLEAN; type != TYPE_DATE; ++type) {
RuntimeState state(TQueryCtx(), ExecEnv::GetInstance());
ObjectPool pool;
@@ -1351,7 +1412,7 @@ TEST_F(ExprTest, NullLiteral) {
}
}
-TEST_F(ExprTest, LiteralConstruction) {
+TEST_P(ExprTest, LiteralConstruction) {
bool b_val = true;
int8_t c_val = 'f';
int16_t s_val = 123;
@@ -1409,7 +1470,7 @@ TEST_F(ExprTest, LiteralConstruction) {
}
-TEST_F(ExprTest, LiteralExprs) {
+TEST_P(ExprTest, LiteralExprs) {
TestFixedPointLimits<int8_t>(TYPE_TINYINT);
TestFixedPointLimits<int16_t>(TYPE_SMALLINT);
TestFixedPointLimits<int32_t>(TYPE_INT);
@@ -1426,7 +1487,7 @@ TEST_F(ExprTest, LiteralExprs) {
}
// IMPALA-3942: Test escaping string literal for single/double quotes
-TEST_F(ExprTest, EscapeStringLiteral) {
+TEST_P(ExprTest, EscapeStringLiteral) {
TestStringValue(R"('"')", R"(")");
TestStringValue(R"("'")", R"(')");
TestStringValue(R"("\"")", R"(")");
@@ -1461,7 +1522,7 @@ TEST_F(ExprTest, EscapeStringLiteral) {
TestStringValue(R"(concat('a\\\'b', "c\\\"d"))", R"(a\'bc\"d)");
}
-TEST_F(ExprTest, ArithmeticExprs) {
+TEST_P(ExprTest, ArithmeticExprs) {
// Test float ops.
TestFixedResultTypeOps<float, float, double>(min_float_values_[TYPE_FLOAT],
min_float_values_[TYPE_FLOAT], TYPE_DOUBLE);
@@ -2968,7 +3029,7 @@ void TestScaleBy() {
}
}
-TEST_F(ExprTest, DecimalArithmeticExprs) {
+TEST_P(ExprTest, DecimalArithmeticExprs) {
// Test with both decimal_v2={false, true}
for (int v2: { 0, 1 }) {
string opt = "DECIMAL_V2=" + lexical_cast<string>(v2);
@@ -3004,7 +3065,7 @@ TEST_F(ExprTest, DecimalArithmeticExprs) {
}
// Tests for expressions that mix decimal and non-decimal arguments with DECIMAL_V2=false.
-TEST_F(ExprTest, DecimalV1MixedArithmeticExprs) {
+TEST_P(ExprTest, DecimalV1MixedArithmeticExprs) {
executor_->PushExecOption("DECIMAL_V2=false");
// IMPALA-3437: decimal constants are implicitly converted to double.
TestValue("10.0 + 3", TYPE_DOUBLE, 13.0);
@@ -3022,7 +3083,7 @@ TEST_F(ExprTest, DecimalV1MixedArithmeticExprs) {
}
// Tests the same expressions as above with DECIMAL_V2=true.
-TEST_F(ExprTest, DecimalV2MixedArithmeticExprs) {
+TEST_P(ExprTest, DecimalV2MixedArithmeticExprs) {
executor_->PushExecOption("DECIMAL_V2=true");
// IMPALA-3437: decimal constants remain decimal.
TestDecimalValue(
@@ -3046,7 +3107,7 @@ TEST_F(ExprTest, DecimalV2MixedArithmeticExprs) {
// There are two tests of ranges, the second of which requires a cast
// of the second operand to a higher-resolution type.
-TEST_F(ExprTest, BinaryPredicates) {
+TEST_P(ExprTest, BinaryPredicates) {
TestComparison("false", "true", false);
TestBinaryPredicates("false", false);
TestBinaryPredicates("true", false);
@@ -3063,7 +3124,7 @@ TEST_F(ExprTest, BinaryPredicates) {
}
// Test casting from all types to all other types
-TEST_F(ExprTest, CastExprs) {
+TEST_P(ExprTest, CastExprs) {
// From tinyint
TestCast("cast(0 as tinyint)", 0);
TestCast("cast(5 as tinyint)", 5);
@@ -3384,7 +3445,7 @@ TEST_F(ExprTest, CastExprs) {
}
// Test casting from/to Date.
-TEST_F(ExprTest, CastDateExprs) {
+TEST_P(ExprTest, CastDateExprs) {
// From Date to Date
TestStringValue("cast(cast(cast('2012-01-01' as date) as date) as string)",
"2012-01-01");
@@ -3510,7 +3571,7 @@ TEST_F(ExprTest, CastDateExprs) {
TestIsNull("cast(cast(null as date) as timestamp)", TYPE_TIMESTAMP);
}
-TEST_F(ExprTest, CompoundPredicates) {
+TEST_P(ExprTest, CompoundPredicates) {
TestValue("TRUE AND TRUE", TYPE_BOOLEAN, true);
TestValue("TRUE AND FALSE", TYPE_BOOLEAN, false);
TestValue("FALSE AND TRUE", TYPE_BOOLEAN, false);
@@ -3561,14 +3622,14 @@ TEST_F(ExprTest, CompoundPredicates) {
TestIsNull("!NULL", TYPE_BOOLEAN);
}
-TEST_F(ExprTest, IsNullPredicate) {
+TEST_P(ExprTest, IsNullPredicate) {
TestValue("5 IS NULL", TYPE_BOOLEAN, false);
TestValue("5 IS NOT NULL", TYPE_BOOLEAN, true);
TestValue("NULL IS NULL", TYPE_BOOLEAN, true);
TestValue("NULL IS NOT NULL", TYPE_BOOLEAN, false);
}
-TEST_F(ExprTest, BoolTestExpr) {
+TEST_P(ExprTest, BoolTestExpr) {
// Tests against constants.
TestValue("TRUE IS TRUE", TYPE_BOOLEAN, true);
TestValue("TRUE IS FALSE", TYPE_BOOLEAN, false);
@@ -3610,7 +3671,7 @@ TEST_F(ExprTest, BoolTestExpr) {
TestValue("(NULL = 1) IS NOT UNKNOWN", TYPE_BOOLEAN, false);
}
-TEST_F(ExprTest, LikePredicate) {
+TEST_P(ExprTest, LikePredicate) {
TestValue("'a' LIKE '%a%'", TYPE_BOOLEAN, true);
TestValue("'a' LIKE '%abcde'", TYPE_BOOLEAN, false);
TestValue("'a' LIKE 'abcde%'", TYPE_BOOLEAN, false);
@@ -3759,7 +3820,7 @@ TEST_F(ExprTest, LikePredicate) {
TestValue("'engLish' IREGEXP 'lIsh$'", TYPE_BOOLEAN, true);
}
-TEST_F(ExprTest, BetweenPredicate) {
+TEST_P(ExprTest, BetweenPredicate) {
// Between is rewritten into a conjunctive compound predicate.
// Compound predicates are also tested elsewere, so we just do basic testing here.
TestValue("5 between 0 and 10", TYPE_BOOLEAN, true);
@@ -3798,7 +3859,7 @@ TEST_F(ExprTest, BetweenPredicate) {
}
// Tests with NULLs are in the FE QueryTest.
-TEST_F(ExprTest, InPredicate) {
+TEST_P(ExprTest, InPredicate) {
// Test integers.
IntValMap::iterator int_iter;
for(int_iter = min_int_values_.begin(); int_iter != min_int_values_.end();
@@ -3908,7 +3969,7 @@ TEST_F(ExprTest, InPredicate) {
TestIsNull("NULL in (NULL, NULL)", TYPE_BOOLEAN);
}
-TEST_F(ExprTest, StringFunctions) {
+TEST_P(ExprTest, StringFunctions) {
TestValue("levenshtein('levenshtein', 'frankenstein')", TYPE_INT, 6);
TestValue("levenshtein('example', 'samples')", TYPE_INT, 3);
TestValue("levenshtein('sturgeon', 'urgently')", TYPE_INT, 6);
@@ -4533,7 +4594,7 @@ TEST_F(ExprTest, StringFunctions) {
TestStringValue(query, big_str);
}
-TEST_F(ExprTest, StringBase64Coding) {
+TEST_P(ExprTest, StringBase64Coding) {
// Test some known values of base64{en,de}code
TestIsNull("base64encode(NULL)", TYPE_STRING);
TestIsNull("base64decode(NULL)", TYPE_STRING);
@@ -4563,7 +4624,7 @@ TEST_F(ExprTest, StringBase64Coding) {
}
}
-TEST_F(ExprTest, LongReverse) {
+TEST_P(ExprTest, LongReverse) {
static const int MAX_LEN = 2048;
string to_reverse(MAX_LEN, ' '), reversed(MAX_LEN, ' ');
// Pick some 'interesting' (i.e. random, but include some powers of two, some primes,
@@ -4575,7 +4636,7 @@ TEST_F(ExprTest, LongReverse) {
}
}
-TEST_F(ExprTest, StringRegexpFunctions) {
+TEST_P(ExprTest, StringRegexpFunctions) {
// Single group.
TestStringValue("regexp_extract('abxcy1234a', 'a.x', 0)", "abx");
TestStringValue("regexp_extract('abxcy1234a', 'a.x.*a', 0)", "abxcy1234a");
@@ -4747,7 +4808,8 @@ TEST_F(ExprTest, StringRegexpFunctions) {
R"('([[:alpha:]]+)(\\\\\\\\)([[:alpha:]]+)', 3))", "world");
}
-TEST_F(ExprTest, StringParseUrlFunction) { // TODO: For now, our parse_url my not behave exactly like Hive
+TEST_P(ExprTest, StringParseUrlFunction) {
+ // TODO: For now, our parse_url my not behave exactly like Hive
// when given malformed URLs.
// If necessary, we can closely follow Java's URL implementation
// to behave exactly like Hive.
@@ -5075,7 +5137,7 @@ TEST_F(ExprTest, StringParseUrlFunction) { // TODO: For now, our parse_url my no
"index.html?test=true&name=networking&op=true', 'XYZ', 'name')");
}
-TEST_F(ExprTest, UtilityFunctions) {
+TEST_P(ExprTest, UtilityFunctions) {
TestStringValue("current_database()", "default");
TestStringValue("current_catalog()", "default");
TestStringValue("user()", "impala_test_user");
@@ -5170,7 +5232,7 @@ TEST_F(ExprTest, UtilityFunctions) {
}
// Test that UtilityFunctions::Coordinator() will return null if coord_address is unset
-TEST_F(ExprTest, CoordinatorFunction) {
+TEST_P(ExprTest, CoordinatorFunction) {
// Make a RuntimeState where the query context does not have coord_address set.
// Note that this should never happen in a real impalad.
RuntimeState state(TQueryCtx(), ExecEnv::GetInstance());
@@ -5189,7 +5251,7 @@ TEST_F(ExprTest, CoordinatorFunction) {
state.ReleaseResources();
}
-TEST_F(ExprTest, MurmurHashFunction) {
+TEST_P(ExprTest, MurmurHashFunction) {
string s("hello world");
int64_t expected = HashUtil::MurmurHash2_64(s.data(), s.size(),
HashUtil::MURMUR_DEFAULT_SEED);
@@ -5239,7 +5301,7 @@ TEST_F(ExprTest, MurmurHashFunction) {
TestIsNull("murmur_hash(NULL)", TYPE_BIGINT);
}
-TEST_F(ExprTest, SessionFunctions) {
+TEST_P(ExprTest, SessionFunctions) {
enum Session {S1, S2};
enum Query {Q1, Q2};
@@ -5257,7 +5319,7 @@ TEST_F(ExprTest, SessionFunctions) {
EXPECT_NE(results[S1][Q1], results[S2][Q1]);
}
-TEST_F(ExprTest, NonFiniteFloats) {
+TEST_P(ExprTest, NonFiniteFloats) {
TestValue("is_inf(0.0)", TYPE_BOOLEAN, false);
TestValue("is_inf(-1/0)", TYPE_BOOLEAN, true);
TestValue("is_inf(1/0)", TYPE_BOOLEAN, true);
@@ -5330,7 +5392,7 @@ TEST_F(ExprTest, NonFiniteFloats) {
TestStringValue("CAST(0/0 AS STRING)", string("nan"));
}
-TEST_F(ExprTest, InvalidFloats) {
+TEST_P(ExprTest, InvalidFloats) {
// IMPALA-1731: Test that leading/trailing garbage is not allowed when parsing inf.
TestIsNull("CAST('1.23inf' AS FLOAT)", TYPE_FLOAT);
TestIsNull("CAST('1.23inf' AS DOUBLE)", TYPE_DOUBLE);
@@ -5374,7 +5436,7 @@ TEST_F(ExprTest, InvalidFloats) {
TestIsNull("CAST('' AS FLOAT)", TYPE_FLOAT);
}
-TEST_F(ExprTest, MathTrigonometricFunctions) {
+TEST_P(ExprTest, MathTrigonometricFunctions) {
// It is important to calculate the expected values
// using math functions, and not simply use constants.
// Otherwise, floating point imprecisions may lead to failed tests.
@@ -5422,7 +5484,7 @@ TEST_F(ExprTest, MathTrigonometricFunctions) {
TestIsNull("degrees(NULL)", TYPE_DOUBLE);
}
-TEST_F(ExprTest, MathConversionFunctions) {
+TEST_P(ExprTest, MathConversionFunctions) {
TestStringValue("bin(0)", "0");
TestStringValue("bin(1)", "1");
TestStringValue("bin(12)", "1100");
@@ -5525,7 +5587,7 @@ TEST_F(ExprTest, MathConversionFunctions) {
TestIsNull("conv(NULL, NULL, NULL)", TYPE_STRING);
}
-TEST_F(ExprTest, MathFunctions) {
+TEST_P(ExprTest, MathFunctions) {
TestValue("pi()", TYPE_DOUBLE, M_PI);
TestValue("e()", TYPE_DOUBLE, M_E);
TestValue("abs(cast(-1.0 as double))", TYPE_DOUBLE, 1.0);
@@ -5949,7 +6011,7 @@ TEST_F(ExprTest, MathFunctions) {
TestIsNull("greatest(cast(NULL as date))", TYPE_DATE);
}
-TEST_F(ExprTest, MathRoundingFunctions) {
+TEST_P(ExprTest, MathRoundingFunctions) {
TestValue("ceil(cast(0.1 as double))", TYPE_DOUBLE, 1);
TestValue("ceil(cast(-10.05 as double))", TYPE_DOUBLE, -10);
TestValue("ceil(cast(23.6 as double))", TYPE_DOUBLE, 24);
@@ -6005,7 +6067,7 @@ TEST_F(ExprTest, MathRoundingFunctions) {
TestIsNull("round(cast(NULL as double), NULL)", TYPE_DOUBLE);
}
-TEST_F(ExprTest, UnaryOperators) {
+TEST_P(ExprTest, UnaryOperators) {
TestValue("+1", TYPE_TINYINT, 1);
TestValue("-1", TYPE_TINYINT, -1);
TestValue("- -1", TYPE_TINYINT, 1);
@@ -6024,7 +6086,7 @@ TEST_F(ExprTest, UnaryOperators) {
TestValue("-1 & 8", TYPE_TINYINT, 8);
}
-TEST_F(ExprTest, MoscowTimezoneConversion) {
+TEST_P(ExprTest, MoscowTimezoneConversion) {
#pragma push_macro("UTC_TO_MSC")
#pragma push_macro("MSC_TO_UTC")
#define UTC_TO_MSC(X) ("cast(from_utc_timestamp('" X "', 'Europe/Moscow') as string)")
@@ -6114,7 +6176,7 @@ void ExprTest::TestAusDSTEndingForCentralTimeZone(const string& time_zone) {
}
// IMPALA-6699: Fix DST end time for Australian time-zones
-TEST_F(ExprTest, AusDSTEndingTests) {
+TEST_P(ExprTest, AusDSTEndingTests) {
TestAusDSTEndingForEastTimeZone("AET");
TestAusDSTEndingForEastTimeZone("Australia/ACT");
TestAusDSTEndingForCentralTimeZone("Australia/Adelaide");
@@ -6131,7 +6193,7 @@ TEST_F(ExprTest, AusDSTEndingTests) {
TestAusDSTEndingForCentralTimeZone("Australia/Yancowinna");
}
-TEST_F(ExprTest, TimestampFunctions) {
+TEST_P(ExprTest, TimestampFunctions) {
// Regression for IMPALA-1105
TestIsNull("cast(cast('NOTATIMESTAMP' as timestamp) as string)", TYPE_STRING);
@@ -7357,7 +7419,7 @@ TEST_F(ExprTest, TimestampFunctions) {
TestIsNull("unix_micros_to_utc_timestamp(253402300800000000)", TYPE_TIMESTAMP);
}
-TEST_F(ExprTest, ConditionalFunctions) {
+TEST_P(ExprTest, ConditionalFunctions) {
// If first param evaluates to true, should return second parameter,
// false or NULL should return the third.
TestValue("if(TRUE, FALSE, TRUE)", TYPE_BOOLEAN, false);
@@ -7653,7 +7715,7 @@ TEST_F(ExprTest, ConditionalFunctions) {
TestIsNull("decode(NULL, 1, 2)", TYPE_TINYINT);
}
-TEST_F(ExprTest, ConditionalFunctionIsTrue) {
+TEST_P(ExprTest, ConditionalFunctionIsTrue) {
TestValue("istrue(cast(false as boolean))", TYPE_BOOLEAN, false);
TestValue("istrue(cast(true as boolean))", TYPE_BOOLEAN, true);
TestValue("istrue(cast(NULL as boolean))", TYPE_BOOLEAN, false);
@@ -7680,7 +7742,7 @@ TEST_F(ExprTest, ConditionalFunctionIsTrue) {
TestError("istrue(-9999999999999999999999999999999999999.9)");
}
-TEST_F(ExprTest, ConditionalFunctionIsFalse) {
+TEST_P(ExprTest, ConditionalFunctionIsFalse) {
TestValue("isfalse(cast(false as boolean))", TYPE_BOOLEAN, true);
TestValue("isfalse(cast(true as boolean))", TYPE_BOOLEAN, false);
TestValue("isfalse(cast(NULL as boolean))", TYPE_BOOLEAN, false);
@@ -7709,7 +7771,7 @@ TEST_F(ExprTest, ConditionalFunctionIsFalse) {
TestError("isfalse(-9999999999999999999999999999999999999.9)");
}
-TEST_F(ExprTest, ConditionalFunctionIsNotTrue) {
+TEST_P(ExprTest, ConditionalFunctionIsNotTrue) {
TestValue("isnottrue(cast(false as boolean))", TYPE_BOOLEAN, true);
TestValue("isnottrue(cast(true as boolean))", TYPE_BOOLEAN, false);
TestValue("isnottrue(cast(NULL as boolean))", TYPE_BOOLEAN, true);
@@ -7738,7 +7800,7 @@ TEST_F(ExprTest, ConditionalFunctionIsNotTrue) {
TestError("isnottrue(-9999999999999999999999999999999999999.9)");
}
-TEST_F(ExprTest, ConditionalFunctionIsNotFalse) {
+TEST_P(ExprTest, ConditionalFunctionIsNotFalse) {
TestValue("isnotfalse(cast(false as boolean))", TYPE_BOOLEAN, false);
TestValue("isnotfalse(cast(true as boolean))", TYPE_BOOLEAN, true);
TestValue("isnotfalse(cast(NULL as boolean))", TYPE_BOOLEAN, true);
@@ -7798,7 +7860,7 @@ void ValidateLayout(const vector<ScalarExpr*>& exprs, int expected_byte_size,
}
}
-TEST_F(ExprTest, ResultsLayoutTest) {
+TEST_P(ExprTest, ResultsLayoutTest) {
vector<ScalarExpr*> exprs;
map<int, set<int>> expected_offsets;
@@ -7932,7 +7994,7 @@ TEST_F(ExprTest, ResultsLayoutTest) {
}
// TODO: is there an easy way to templatize/parametrize these tests?
-TEST_F(ExprTest, DecimalFunctions) {
+TEST_P(ExprTest, DecimalFunctions) {
TestValue("precision(cast (1 as decimal(10,2)))", TYPE_INT, 10);
TestValue("scale(cast(1 as decimal(10,2)))", TYPE_INT, 2);
@@ -8455,7 +8517,7 @@ TEST_F(ExprTest, DecimalFunctions) {
// Sanity check some overflow casting. We have a random test framework that covers
// this more thoroughly.
-TEST_F(ExprTest, DecimalOverflowCastsDecimalV1) {
+TEST_P(ExprTest, DecimalOverflowCastsDecimalV1) {
executor_->PushExecOption("DECIMAL_V2=0");
TestDecimalValue("cast(123.456 as decimal(6,3))",
@@ -8530,7 +8592,7 @@ TEST_F(ExprTest, DecimalOverflowCastsDecimalV1) {
executor_->PopExecOption();
}
-TEST_F(ExprTest, DecimalOverflowCastsDecimalV2) {
+TEST_P(ExprTest, DecimalOverflowCastsDecimalV2) {
executor_->PushExecOption("DECIMAL_V2=1");
TestDecimalValue("cast(123.456 as decimal(6,3))",
@@ -8599,7 +8661,7 @@ TEST_F(ExprTest, DecimalOverflowCastsDecimalV2) {
executor_->PopExecOption();
}
-TEST_F(ExprTest, NullValueFunction) {
+TEST_P(ExprTest, NullValueFunction) {
TestValue("nullvalue(cast(NULL as boolean))", TYPE_BOOLEAN, true);
TestValue("nullvalue(cast(0 as boolean))", TYPE_BOOLEAN, false);
TestValue("nullvalue(cast(5 as boolean))", TYPE_BOOLEAN, false);
@@ -8669,7 +8731,7 @@ TEST_F(ExprTest, NullValueFunction) {
TestValue("nullvalue(-99999999999999999999999999999999999.9)", TYPE_BOOLEAN, false);
}
-TEST_F(ExprTest, NonNullValueFunction) {
+TEST_P(ExprTest, NonNullValueFunction) {
TestValue("nonnullvalue(cast(NULL as boolean))", TYPE_BOOLEAN, false);
TestValue("nonnullvalue(cast(0 as boolean))", TYPE_BOOLEAN, true);
TestValue("nonnullvalue(cast(5 as boolean))", TYPE_BOOLEAN, true);
@@ -8739,7 +8801,7 @@ TEST_F(ExprTest, NonNullValueFunction) {
TestValue("nonnullvalue(-99999999999999999999999999999999999.9)", TYPE_BOOLEAN, true);
}
-TEST_F(ExprTest, UdfInterfaceBuiltins) {
+TEST_P(ExprTest, UdfInterfaceBuiltins) {
TestValue("udf_pi()", TYPE_DOUBLE, M_PI);
TestValue("udf_abs(-1)", TYPE_DOUBLE, 1.0);
TestStringValue("udf_lower('Hello_WORLD')", "hello_world");
@@ -8755,7 +8817,7 @@ TEST_F(ExprTest, UdfInterfaceBuiltins) {
TestValue("min_bigint()", TYPE_BIGINT, numeric_limits<int64_t>::min());
}
-TEST_F(ExprTest, MADlib) {
+TEST_P(ExprTest, MADlib) {
TestStringValue("madlib_encode_vector(madlib_vector(1.0, 2.0, 3.0))",
"aaaaaipdaaaaaaaeaaaaaeae");
TestStringValue("madlib_print_vector(madlib_vector(1, 2, 3))", "<1, 2, 3>");
@@ -8774,7 +8836,7 @@ TEST_F(ExprTest, MADlib) {
TYPE_DOUBLE, 3.0);
}
-TEST_F(ExprTest, BitByteBuiltins) {
+TEST_P(ExprTest, BitByteBuiltins) {
TestIsNull("bitand(1,NULL)", TYPE_TINYINT);
TestIsNull("bitand(NULL,3)", TYPE_TINYINT);
// And of numbers differing in 2nd bit position gives min of two numbers
@@ -8945,7 +9007,7 @@ TEST_F(ExprTest, BitByteBuiltins) {
TestValue("rotateright(256, -2)", TYPE_SMALLINT, 1024);
}
-TEST_F(ExprTest, UuidTest) {
+TEST_P(ExprTest, UuidTest) {
boost::unordered_set<string> string_set;
const unsigned int NUM_UUIDS = 10;
const string regex(
@@ -8957,7 +9019,7 @@ TEST_F(ExprTest, UuidTest) {
EXPECT_TRUE(string_set.size() == NUM_UUIDS);
}
-TEST_F(ExprTest, DateTruncTest) {
+TEST_P(ExprTest, DateTruncTest) {
TestTimestampValue("date_trunc('MILLENNIUM', '2016-05-08 10:30:00')",
TimestampValue::Parse("2001-01-01 00:00:00"));
TestTimestampValue("date_trunc('MILLENNIUM', '3000-12-31 23:59:59.999999999')",
@@ -9081,7 +9143,7 @@ TEST_F(ExprTest, DateTruncTest) {
TestError("date_trunc('2017-01-09 10:00:00', 'HOUR')");
}
-TEST_F(ExprTest, JsonTest) {
+TEST_P(ExprTest, JsonTest) {
TestStringValue("get_json_object('{\"a\":1, \"b\":2, \"c\":3}', '$.b')", "2");
TestStringValue(
"get_json_object('{\"a\":true, \"b\":false, \"c\":true}', '$.b')", "false");
@@ -9225,77 +9287,25 @@ TEST_F(ExprTest, JsonTest) {
}
} // namespace impala
+INSTANTIATE_TEST_CASE_P(Instantiations, ExprTest, ::testing::Values(
+ // disable_codegen enable_expr_rewrites
+ std::make_tuple(true, false),
+ std::make_tuple(false, false),
+ std::make_tuple(true, true)));
+ // Note: the false/true case is not tested because it provides very little
+ // incremental coverage but adds a lot to test runtime (this test is quite
+ // slow when codegen is enabled).
+ //
+ // Mostly we get the best backend codegened/interpreted coverage from running
+ // the unmodified (i.e. more complex) expr trees enabled. Running the trees
+ // in interpreted mode should be enough to validate correctness of the
+ // rewrites. So enabling the remaining combination might provide some
+ // additional incidental coverage from codegening some additional expr tree
+ // shapes but mostly it isn't that interesting since the majority of
+ // expressions get folded to a constant anyway.
+
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
InitCommonRuntime(argc, argv, true, TestInfo::BE_TEST);
- InitFeSupport(false);
- ABORT_IF_ERROR(impala::LlvmCodeGen::InitializeLlvm());
-
- // The host running this test might have an out-of-date tzdata package installed.
- // To avoid tzdata related issues, we will load time-zone db from the testdata
- // directory.
- FLAGS_hdfs_zone_info_zip = Substitute("file://$0/testdata/tzdb/2017c.zip",
- getenv("IMPALA_HOME"));
- ABORT_IF_ERROR(TimezoneDatabase::Initialize());
-
- // Disable llvm optimization passes if the env var is no set to true. Running without
- // the optimizations makes the tests run much faster.
- char* optimizations = getenv("EXPR_TEST_ENABLE_OPTIMIZATIONS");
- if (optimizations != NULL && strcmp(optimizations, "true") == 0) {
- cout << "Running with optimization passes." << endl;
- FLAGS_disable_optimization_passes = false;
- } else {
- cout << "Running without optimization passes." << endl;
- FLAGS_disable_optimization_passes = true;
- }
-
- // Create an in-process Impala server and in-process backends for test environment
- // without any startup validation check
- FLAGS_abort_on_config_error = false;
- VLOG_CONNECTION << "creating test env";
- VLOG_CONNECTION << "starting backends";
- statestore = new Statestore(statestore_metrics.get());
- IGNORE_LEAKING_OBJECT(statestore);
-
- // Pass in 0 to have the statestore use an ephemeral port for the service.
- ABORT_IF_ERROR(statestore->Init(0));
- InProcessImpalaServer* impala_server;
- ABORT_IF_ERROR(InProcessImpalaServer::StartWithEphemeralPorts(
- FLAGS_hostname, statestore->port(), &impala_server));
- IGNORE_LEAKING_OBJECT(impala_server);
-
- executor_ = new ImpaladQueryExecutor(FLAGS_hostname, impala_server->GetBeeswaxPort());
- ABORT_IF_ERROR(executor_->Setup());
-
- // Disable FE expr rewrites to make sure the Exprs get executed exactly as specified
- // in the tests here.
- int ret;
- disable_codegen_ = true;
- enable_expr_rewrites_ = false;
- executor_->ClearExecOptions();
- executor_->PushExecOption("ENABLE_EXPR_REWRITES=0");
- executor_->PushExecOption("DISABLE_CODEGEN=1");
- cout << "Running without codegen and without expr rewrites" << endl;
- ret = RUN_ALL_TESTS();
- if (ret != 0) return ret;
-
- disable_codegen_ = false;
- enable_expr_rewrites_ = false;
- executor_->ClearExecOptions();
- executor_->PushExecOption("ENABLE_EXPR_REWRITES=0");
- executor_->PushExecOption("DISABLE_CODEGEN=0");
- executor_->PushExecOption("EXEC_SINGLE_NODE_ROWS_THRESHOLD=0");
- executor_->PushExecOption("DISABLE_CODEGEN_ROWS_THRESHOLD=0");
- cout << endl << "Running with codegen and without expr rewrites" << endl;
- ret = RUN_ALL_TESTS();
- if (ret != 0) return ret;
-
- // Enable FE expr rewrites to get test for constant folding over all exprs.
- disable_codegen_ = true;
- enable_expr_rewrites_ = true;
- executor_->ClearExecOptions();
- executor_->PushExecOption("ENABLE_EXPR_REWRITES=1");
- executor_->PushExecOption("DISABLE_CODEGEN=1");
- cout << endl << "Running without codegen and with expr rewrites" << endl;
return RUN_ALL_TESTS();
}
[impala] 01/03: IMPALA-8281: Misc Sentry decoupling clean up
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 6140c1c5af1c6a33804c6be8683e88c955d2a47a
Author: Austin Nobis <an...@cloudera.com>
AuthorDate: Thu May 2 15:52:59 2019 -0500
IMPALA-8281: Misc Sentry decoupling clean up
This patch moves Sentry specific code to the Sentry specific plugin
implementation.
Testing:
- Ran all FE tests
- Ran E2E authorization tests
- Added new E2E tests in test_sentry
Change-Id: Id24a00dd395e30e4c392f085893e9561da2ee539
Reviewed-on: http://gerrit.cloudera.org:8080/13284
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
common/thrift/Frontend.thrift | 4 +-
.../apache/impala/analysis/CreateDropRoleStmt.java | 6 ---
.../impala/analysis/GrantRevokePrivStmt.java | 14 +----
.../impala/analysis/GrantRevokeRoleStmt.java | 3 --
.../impala/analysis/ShowGrantPrincipalStmt.java | 24 ---------
.../org/apache/impala/analysis/ShowRolesStmt.java | 1 -
.../sentry/SentryCatalogdAuthorizationManager.java | 62 +++++++++++++++++-----
.../sentry/SentryImpaladAuthorizationManager.java | 44 ++++++++++++++-
.../java/org/apache/impala/service/Frontend.java | 31 +----------
.../impala/analysis/AnalyzeAuthStmtsTest.java | 13 -----
tests/authorization/test_sentry.py | 42 +++++++++++++++
11 files changed, 140 insertions(+), 104 deletions(-)
diff --git a/common/thrift/Frontend.thrift b/common/thrift/Frontend.thrift
index 3f0075b..2599910 100644
--- a/common/thrift/Frontend.thrift
+++ b/common/thrift/Frontend.thrift
@@ -264,7 +264,7 @@ struct TShowRolesParams {
// True if this opertion requires admin privileges on the Sentry Service. This is
// needed to check for the case where an operation is_user_scope, but the user does
// not belong to the specified grant_group.
- 2: required bool is_admin_op
+ // REMOVED: 2: required bool is_admin_op
// True if the statement is "SHOW CURRENT ROLES".
3: required bool is_show_current_roles
@@ -292,7 +292,7 @@ struct TShowGrantPrincipalParams {
// True if this operation requires admin privileges on the Sentry Service (when
// the requesting user has not been granted the target role name).
- 4: required bool is_admin_op
+ // REMOVED: 4: required bool is_admin_op
// An optional filter to show grants that match a specific privilege spec.
5: optional CatalogObjects.TPrivilege privilege
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
index fb18d41..e85c0ab 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
@@ -51,11 +51,5 @@ public class CreateDropRoleStmt extends AuthorizationStmt {
@Override
public void analyze(Analyzer analyzer) throws AnalysisException {
super.analyze(analyzer);
- Role existingRole = analyzer.getCatalog().getAuthPolicy().getRole(roleName_);
- if (isDropRole_ && existingRole == null) {
- throw new AnalysisException(String.format("Role '%s' does not exist.", roleName_));
- } else if (!isDropRole_ && existingRole != null) {
- throw new AnalysisException(String.format("Role '%s' already exists.", roleName_));
- }
}
}
\ No newline at end of file
diff --git a/fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java b/fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
index d2e285a..0fe898e 100644
--- a/fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
@@ -33,8 +33,8 @@ import com.google.common.base.Strings;
* All privilege checks on catalog objects are skipped when executing
* GRANT/REVOKE statements. This is because we need to be able to create
* privileges on an object before any privileges actually exist.
- * The GRANT/REVOKE statement itself will be authorized (currently by
- * the Sentry Service).
+ * The GRANT/REVOKE statement itself will be authorized by the loaded
+ * authorization provider.
*/
public class GrantRevokePrivStmt extends AuthorizationStmt {
private final PrivilegeSpec privilegeSpec_;
@@ -43,10 +43,6 @@ public class GrantRevokePrivStmt extends AuthorizationStmt {
private final boolean hasGrantOpt_;
private final TPrincipalType principalType_;
- // Set/modified during analysis
- // TODO: This will need to be cleaned up when Ranger supports RBAC
- private Role role_;
-
public GrantRevokePrivStmt(String roleName, PrivilegeSpec privilegeSpec,
boolean isGrantPrivStmt, boolean hasGrantOpt, TPrincipalType principalType) {
Preconditions.checkNotNull(privilegeSpec);
@@ -64,9 +60,6 @@ public class GrantRevokePrivStmt extends AuthorizationStmt {
params.setIs_grant(isGrantPrivStmt_);
List<TPrivilege> privileges = privilegeSpec_.toThrift();
for (TPrivilege privilege: privileges) {
- if (principalType_ == TPrincipalType.ROLE && role_ != null) {
- privilege.setPrincipal_id(role_.getId());
- }
privilege.setPrincipal_type(principalType_);
privilege.setHas_grant_opt(hasGrantOpt_);
}
@@ -102,9 +95,6 @@ public class GrantRevokePrivStmt extends AuthorizationStmt {
"empty.");
}
- if (principalType_ == TPrincipalType.ROLE) {
- role_ = analyzer.getCatalog().getAuthPolicy().getRole(principalName_);
- }
privilegeSpec_.analyze(analyzer);
}
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java b/fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
index c7aef59..06d9347 100644
--- a/fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
@@ -57,9 +57,6 @@ public class GrantRevokeRoleStmt extends AuthorizationStmt {
@Override
public void analyze(Analyzer analyzer) throws AnalysisException {
super.analyze(analyzer);
- if (analyzer.getCatalog().getAuthPolicy().getRole(roleName_) == null) {
- throw new AnalysisException(String.format("Role '%s' does not exist.", roleName_));
- }
if (Strings.isNullOrEmpty(roleName_)) {
throw new AnalysisException("Role name in GRANT/REVOKE ROLE cannot be empty.");
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java b/fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
index 34f15b7..103dde6 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
@@ -37,9 +37,6 @@ public class ShowGrantPrincipalStmt extends AuthorizationStmt {
private final String name_;
private final TPrincipalType principalType_;
- // Set/modified during analysis.
- private Principal principal_;
-
public ShowGrantPrincipalStmt(String name, TPrincipalType principalType,
PrivilegeSpec privilegeSpec) {
Preconditions.checkNotNull(name);
@@ -58,24 +55,6 @@ public class ShowGrantPrincipalStmt extends AuthorizationStmt {
Principal.toString(principalType_).toUpperCase()));
}
- switch(principalType_) {
- case ROLE:
- principal_ = analyzer.getCatalog().getAuthPolicy().getPrincipal(name_,
- principalType_);
- if (principal_ == null) {
- throw new AnalysisException(String.format("%s '%s' " +
- "does not exist.", Principal.toString(principalType_), name_));
- }
- break;
- case USER:
- case GROUP:
- principal_ = Principal.newInstance(name_, principalType_, new HashSet<>());
- break;
- default:
- throw new AnalysisException(String.format("Unexpected TPrincipalType %s",
- principalType_.name()));
- }
-
if (privilegeSpec_ != null) privilegeSpec_.analyze(analyzer);
}
@@ -118,10 +97,7 @@ public class ShowGrantPrincipalStmt extends AuthorizationStmt {
params.setRequesting_user(requestingUser_.getShortName());
if (privilegeSpec_ != null) {
params.setPrivilege(privilegeSpec_.toThrift().get(0));
- params.getPrivilege().setPrincipal_id(principal_.getId());
}
return params;
}
-
- public Principal getPrincipal() { return principal_; }
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java b/fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
index ded1445..8d4d62f 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
@@ -60,7 +60,6 @@ public class ShowRolesStmt extends AuthorizationStmt {
params.setIs_show_current_roles(isShowCurrentRoles_);
if (groupName_ != null) params.setGrant_group(groupName_);
// Users should always be able to execute SHOW CURRENT ROLES.
- params.setIs_admin_op(!isShowCurrentRoles_);
return params;
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java
index 0c3ca3c..ffe9000 100644
--- a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java
@@ -21,9 +21,11 @@ import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import org.apache.hadoop.hive.metastore.api.PrincipalType;
import org.apache.impala.authorization.AuthorizationDelta;
+import org.apache.impala.authorization.AuthorizationException;
import org.apache.impala.authorization.AuthorizationManager;
import org.apache.impala.authorization.User;
import org.apache.impala.catalog.CatalogException;
+import org.apache.impala.catalog.CatalogObject;
import org.apache.impala.catalog.CatalogServiceCatalog;
import org.apache.impala.catalog.Principal;
import org.apache.impala.catalog.PrincipalPrivilege;
@@ -51,6 +53,7 @@ import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.List;
+import java.util.stream.Collectors;
/**
* An implementation of {@link AuthorizationManager} for Catalogd that uses Sentry.
@@ -90,7 +93,13 @@ public class SentryCatalogdAuthorizationManager implements AuthorizationManager
TDdlExecResponse response) throws ImpalaException {
verifySentryServiceEnabled();
- Role role = sentryProxy_.createRole(requestingUser, params.getRole_name());
+ Role role = catalog_.getAuthPolicy().getRole(params.getRole_name());
+ if (role != null) {
+ throw new AuthorizationException(String.format("Role '%s' already exists.",
+ params.getRole_name()));
+ }
+
+ role = sentryProxy_.createRole(requestingUser, params.getRole_name());
Preconditions.checkNotNull(role);
TCatalogObject catalogObject = new TCatalogObject();
@@ -106,7 +115,13 @@ public class SentryCatalogdAuthorizationManager implements AuthorizationManager
TDdlExecResponse response) throws ImpalaException {
verifySentryServiceEnabled();
- Role role = sentryProxy_.dropRole(requestingUser, params.getRole_name());
+ Role role = catalog_.getAuthPolicy().getRole(params.getRole_name());
+ if (role == null) {
+ throw new AuthorizationException(String.format("Role '%s' does not exist.",
+ params.getRole_name()));
+ }
+
+ role = sentryProxy_.dropRole(requestingUser, params.getRole_name());
if (role == null) {
// Nothing was removed from the catalogd's cache.
response.result.setVersion(catalog_.getCatalogVersion());
@@ -135,15 +150,16 @@ public class SentryCatalogdAuthorizationManager implements AuthorizationManager
Preconditions.checkArgument(!params.getGroup_names().isEmpty());
verifySentryServiceEnabled();
+ if (catalog_.getAuthPolicy().getRole(params.getRole_names().get(0)) == null) {
+ throw new AuthorizationException(String.format("Role '%s' does not exist.",
+ params.getRole_names().get(0)));
+ }
+
String roleName = params.getRole_names().get(0);
String groupName = params.getGroup_names().get(0);
Role role = sentryProxy_.grantRoleGroup(requestingUser, roleName, groupName);
Preconditions.checkNotNull(role);
- TCatalogObject catalogObject = new TCatalogObject();
- catalogObject.setType(role.getCatalogObjectType());
- catalogObject.setPrincipal(role.toThrift());
- catalogObject.setCatalog_version(role.getCatalogVersion());
- response.result.addToUpdated_catalog_objects(catalogObject);
+ response.result.addToUpdated_catalog_objects(createRoleObject(role));
response.result.setVersion(role.getCatalogVersion());
}
@@ -154,16 +170,26 @@ public class SentryCatalogdAuthorizationManager implements AuthorizationManager
Preconditions.checkArgument(!params.getGroup_names().isEmpty());
verifySentryServiceEnabled();
+ if (catalog_.getAuthPolicy().getRole(params.getRole_names().get(0)) == null) {
+ throw new AuthorizationException(String.format("Role '%s' does not exist.",
+ params.getRole_names().get(0)));
+ }
+
String roleName = params.getRole_names().get(0);
String groupName = params.getGroup_names().get(0);
Role role = sentryProxy_.revokeRoleGroup(requestingUser, roleName, groupName);
+ response.result.addToUpdated_catalog_objects(createRoleObject(role));
+ response.result.setVersion(role.getCatalogVersion());
+ }
+
+ private static TCatalogObject createRoleObject(Role role) {
Preconditions.checkNotNull(role);
+
TCatalogObject catalogObject = new TCatalogObject();
catalogObject.setType(role.getCatalogObjectType());
catalogObject.setPrincipal(role.toThrift());
catalogObject.setCatalog_version(role.getCatalogVersion());
- response.result.addToUpdated_catalog_objects(catalogObject);
- response.result.setVersion(role.getCatalogVersion());
+ return catalogObject;
}
@Override
@@ -174,11 +200,13 @@ public class SentryCatalogdAuthorizationManager implements AuthorizationManager
String roleName = params.getPrincipal_name();
Role role = catalog_.getAuthPolicy().getRole(roleName);
if (role == null) {
- throw new InternalException(String.format("Role '%s' does not exists.",
+ throw new AuthorizationException(String.format("Role '%s' does not exist.",
roleName));
}
- List<TPrivilege> privileges = params.getPrivileges();
+ List<TPrivilege> privileges = params.getPrivileges().stream()
+ .peek(p -> p.setPrincipal_id(role.getId()))
+ .collect(Collectors.toList());
List<PrincipalPrivilege> removedGrantOptPrivileges =
Lists.newArrayListWithExpectedSize(privileges.size());
List<PrincipalPrivilege> addedRolePrivileges =
@@ -216,8 +244,18 @@ public class SentryCatalogdAuthorizationManager implements AuthorizationManager
verifySentryServiceEnabled();
Preconditions.checkArgument(!params.getPrivileges().isEmpty());
+ Role role = catalog_.getAuthPolicy().getRole(params.principal_name);
+ if (role == null) {
+ throw new AuthorizationException(String.format("Role '%s' does not exist.",
+ params.getPrincipal_name()));
+ }
+
String roleName = params.getPrincipal_name();
- List<TPrivilege> privileges = params.getPrivileges();
+ List<TPrivilege> privileges = params.getPrivileges().stream()
+ .peek(p -> {
+ if (role != null) p.setPrincipal_id(role.getId());
+ }).collect(Collectors.toList());
+
// If this is a revoke of a privilege that contains the grant option, the privileges
// with the grant option will be revoked and new privileges without the grant option
// will be added. The privilege in the catalog cannot simply be updated since the
diff --git a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java
index 92c13b3..e4bdfba 100644
--- a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java
@@ -27,6 +27,7 @@ import org.apache.impala.authorization.AuthorizationDelta;
import org.apache.impala.authorization.AuthorizationException;
import org.apache.impala.authorization.AuthorizationManager;
import org.apache.impala.authorization.User;
+import org.apache.impala.catalog.Principal;
import org.apache.impala.catalog.Role;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.InternalException;
@@ -50,10 +51,13 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Collections;
+import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.function.Supplier;
+import static org.apache.impala.thrift.TPrincipalType.ROLE;
+
/**
* An implementation of {@link AuthorizationManager} for Impalad that uses Sentry.
*
@@ -95,7 +99,18 @@ public class SentryImpaladAuthorizationManager implements AuthorizationManager {
@Override
public TShowRolesResult getRoles(TShowRolesParams params) throws ImpalaException {
- if (params.isIs_admin_op()) {
+ Set<String> groups = authzChecker_.get()
+ .getUserGroups(new User(params.requesting_user));
+
+ // Check if the user is part of the group (case-sensitive) this SHOW ROLES
+ // statement is targeting. If they are already a member of the group,
+ // the admin requirement can be removed.
+ // If the the statement is SHOW CURRENT ROLES, the admin requirement can also be
+ // removed.
+ boolean adminOp =
+ !(groups.contains(params.getGrant_group()) || params.is_show_current_roles);
+
+ if (adminOp) {
validateSentryAdmin(params.getRequesting_user());
}
@@ -187,7 +202,17 @@ public class SentryImpaladAuthorizationManager implements AuthorizationManager {
@Override
public TResultSet getPrivileges(TShowGrantPrincipalParams params)
throws ImpalaException {
- if (params.isIs_admin_op()) {
+ Principal principal = (params.principal_type == ROLE) ?
+ catalog_.getOrCreateCatalog().getAuthPolicy().getPrincipal(params.getName(),
+ params.getPrincipal_type()) :
+ Principal.newInstance(params.name, params.principal_type, new HashSet<>());
+
+ if (principal == null) {
+ throw new AuthorizationException(String.format("%s '%s' does not exist.",
+ Principal.toString(params.principal_type), params.name));
+ }
+
+ if (isAdminOp(params, principal, authzChecker_.get())) {
validateSentryAdmin(params.getRequesting_user());
}
@@ -206,6 +231,21 @@ public class SentryImpaladAuthorizationManager implements AuthorizationManager {
}
}
+ private static boolean isAdminOp(TShowGrantPrincipalParams params, Principal principal,
+ SentryAuthorizationChecker authzChecker) throws ImpalaException {
+ Set<String> groupNames = authzChecker.getUserGroups(new User(params.requesting_user));
+
+ switch (params.principal_type) {
+ case USER:
+ return !principal.getName().equals(params.requesting_user);
+ case GROUP:
+ case ROLE:
+ return Sets.intersection(groupNames, principal.getGrantGroups()).isEmpty();
+ default:
+ return false;
+ }
+ }
+
@Override
public void updateDatabaseOwnerPrivilege(String serverName, String databaseName,
String oldOwner, PrincipalType oldOwnerType, String newOwner,
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index cb9f9ab..1f5c803 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -499,24 +499,12 @@ public class Frontend {
ResetMetadataStmt resetMetadataStmt = (ResetMetadataStmt) analysis.getStmt();
TResetMetadataRequest req = resetMetadataStmt.toThrift();
ddl.setReset_metadata_params(req);
- metadata.setColumns(Collections.<TColumn>emptyList());
+ metadata.setColumns(Collections.emptyList());
} else if (analysis.isShowRolesStmt()) {
ddl.op_type = TCatalogOpType.SHOW_ROLES;
ShowRolesStmt showRolesStmt = (ShowRolesStmt) analysis.getStmt();
ddl.setShow_roles_params(showRolesStmt.toThrift());
Preconditions.checkState(getAuthzChecker() instanceof SentryAuthorizationChecker);
- Set<String> groupNames = getAuthzChecker().getUserGroups(
- analysis.getAnalyzer().getUser());
- // Check if the user is part of the group (case-sensitive) this SHOW ROLE
- // statement is targeting. If they are already a member of the group,
- // the admin requirement can be removed.
- // If the the statement is SHOW CURRENT ROLES, the admin requirement can also be
- // removed.
- Preconditions.checkState(ddl.getShow_roles_params().isSetIs_admin_op());
- ddl.getShow_roles_params().setIs_admin_op(!(
- (ddl.getShow_roles_params().isSetGrant_group() &&
- groupNames.contains(ddl.getShow_roles_params().getGrant_group())) ||
- ddl.getShow_roles_params().isIs_show_current_roles()));
metadata.setColumns(Arrays.asList(
new TColumn("role_name", Type.STRING.toThrift())));
} else if (analysis.isShowGrantPrincipalStmt()) {
@@ -524,22 +512,7 @@ public class Frontend {
ShowGrantPrincipalStmt showGrantPrincipalStmt =
(ShowGrantPrincipalStmt) analysis.getStmt();
ddl.setShow_grant_principal_params(showGrantPrincipalStmt.toThrift());
- Set<String> groupNames = getAuthzChecker().getUserGroups(
- analysis.getAnalyzer().getUser());
- // User must be an admin to execute this operation if they have not been granted
- // this principal, or the same user as the request.
- boolean requiresAdmin;
- if (showGrantPrincipalStmt.getPrincipal().getPrincipalType()
- == TPrincipalType.USER) {
- requiresAdmin = !showGrantPrincipalStmt.getPrincipal().getName().equals(
- analysis.getAnalyzer().getUser().getShortName());
- } else {
- requiresAdmin = Sets.intersection(groupNames, showGrantPrincipalStmt
- .getPrincipal().getGrantGroups()).isEmpty();
- }
- ddl.getShow_grant_principal_params().setIs_admin_op(requiresAdmin);
- metadata.setColumns(Arrays.asList(
- new TColumn("name", Type.STRING.toThrift())));
+ metadata.setColumns(Arrays.asList(new TColumn("name", Type.STRING.toThrift())));
} else if (analysis.isCreateDropRoleStmt()) {
CreateDropRoleStmt createDropRoleStmt = (CreateDropRoleStmt) analysis.getStmt();
TCreateDropRoleParams params = createDropRoleStmt.toThrift();
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
index e3ce330..1532965 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
@@ -116,11 +116,6 @@ public class AnalyzeAuthStmtsTest extends FrontendTestBase {
AnalysisError("SHOW GRANT ROLE myRole ON SERVER", authDisabledCtx,
"Authorization is not enabled.");
}
- AnalysisError("SHOW GRANT ROLE does_not_exist",
- "Role 'does_not_exist' does not exist.");
-
- AnalysisError("SHOW GRANT ROLE does_not_exist ON SERVER",
- "Role 'does_not_exist' does not exist.");
// Determining if a user exists on the system is done in the AuthorizationPolicy and
// these tests run with authorization disabled. The SHOW GRANT USER will be tested
@@ -132,12 +127,8 @@ public class AnalyzeAuthStmtsTest extends FrontendTestBase {
AnalyzesOk("DROP ROLE myRole");
AnalyzesOk("CREATE ROLE doesNotExist");
- AnalysisError("DROP ROLE doesNotExist", "Role 'doesNotExist' does not exist.");
- AnalysisError("CREATE ROLE myRole", "Role 'myRole' already exists.");
-
// Role names are case-insensitive
AnalyzesOk("DROP ROLE MYrole");
- AnalysisError("CREATE ROLE MYrole", "Role 'MYrole' already exists.");
AnalysisContext authDisabledCtx = createAuthDisabledAnalysisCtx();
AnalysisError("DROP ROLE myRole", authDisabledCtx,
@@ -150,10 +141,6 @@ public class AnalyzeAuthStmtsTest extends FrontendTestBase {
public void AnalyzeGrantRevokeRole() throws AnalysisException {
AnalyzesOk("GRANT ROLE myrole TO GROUP abc");
AnalyzesOk("REVOKE ROLE myrole FROM GROUP abc");
- AnalysisError("GRANT ROLE doesNotExist TO GROUP abc",
- "Role 'doesNotExist' does not exist.");
- AnalysisError("REVOKE ROLE doesNotExist FROM GROUP abc",
- "Role 'doesNotExist' does not exist.");
AnalysisContext authDisabledCtx = createAuthDisabledAnalysisCtx();
AnalysisError("GRANT ROLE myrole TO GROUP abc", authDisabledCtx,
diff --git a/tests/authorization/test_sentry.py b/tests/authorization/test_sentry.py
index 494fdb5..f650b1d 100644
--- a/tests/authorization/test_sentry.py
+++ b/tests/authorization/test_sentry.py
@@ -15,11 +15,13 @@
# specific language governing permissions and limitations
# under the License.
+import grp
import pytest
import os
from getpass import getuser
from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
+from tests.common.sentry_cache_test_suite import SentryCacheTestSuite
SENTRY_CONFIG_DIR = os.getenv('IMPALA_HOME') + '/fe/src/test/resources/'
SENTRY_CONFIG_FILE = SENTRY_CONFIG_DIR + 'sentry-site.xml'
@@ -58,3 +60,43 @@ class TestSentry(CustomClusterTestSuite):
"the requested policy metadata.".format(non_admin) in str(result)
finally:
admin_client.execute("drop role {0}".format(unique_role))
+
+ @pytest.mark.execute_serially
+ @SentryCacheTestSuite.with_args(
+ impalad_args="--server_name=server1",
+ catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE,
+ sentry_config=SENTRY_CONFIG_FILE)
+ def test_grant_revoke_invalid_role(self, unique_name):
+ role_name = "foobar"
+ group = grp.getgrnam(getuser()).gr_name
+ try:
+ # This will create "foobar" role catalog object.
+ self.client.execute("create role {0}".format(role_name))
+ self.client.execute("grant all on server to {0}".format(role_name))
+ self.client.execute("grant role {0} to group `{1}`".format(role_name, group))
+ self.client.execute("create database {0}".format(unique_name))
+
+ ex = self.execute_query_expect_failure(
+ self.client, "grant all on database {0} to role non_role".format(unique_name))
+ assert "Role 'non_role' does not exist." in str(ex)
+
+ ex = self.execute_query_expect_failure(
+ self.client, "revoke all on database {0} from role non_role".format(unique_name))
+ assert "Role 'non_role' does not exist." in str(ex)
+
+ ex = self.execute_query_expect_failure(self.client, "show grant role non_role")
+ assert "Role 'non_role' does not exist." in str(ex)
+
+ ex = self.execute_query_expect_failure(
+ self.client, "grant role non_role to group `{0}`".format(group))
+ assert "Role 'non_role' does not exist." in str(ex)
+
+ ex = self.execute_query_expect_failure(self.client, "drop role non_role")
+ assert "Role 'non_role' does not exist." in str(ex)
+
+ ex = self.execute_query_expect_failure(self.client,
+ "create role {0}".format(role_name))
+ assert "Role '{0}' already exists.".format(role_name) in str(ex)
+ finally:
+ self.client.execute("drop database {0}".format(unique_name))
+ self.client.execute("drop role {0}".format(role_name))
[impala] 03/03: IMPALA-7665: Fix unwarranted query cancellation on
statestore restart
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit e4352aa63f2bdfb0f9e82f8b04567fa6b729af95
Author: Bikramjeet Vig <bi...@cloudera.com>
AuthorDate: Tue Apr 16 15:49:18 2019 -0700
IMPALA-7665: Fix unwarranted query cancellation on statestore restart
Currently, if the statestore restarts and disseminates an inconsistent
view of cluster membership to the coordinators, then they might believe
that the backends no longer in the membership update are down and would
start canceling queries that are running or scheduled to run on those
allegedly failed backends. This patch adds a grace period after
statestore recovery/successful registration that give it enough time
to gather a consistent state of the cluster.
Testing:
- Added an e2e test.
- Did manual stress testing using concurrent_select.py with
statestore_subscriber_timeout_seconds set to 2 secs and
failed_backends_query_cancellation_grace_period_ms set to 5 seconds,
and the statestore being restarted every 15 seconds. To avoid other
effects of statestore restarts cropping up, I used a local catalog
(catalog v2) and ignored query errors caused due to scheduler having
an incomplete view of the cluster(no backends).
Change-Id: I30b68bd8bde4bf589d58d42d6f683afb166de959
Reviewed-on: http://gerrit.cloudera.org:8080/13061
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/service/impala-server.cc | 13 ++++-
be/src/statestore/statestore-subscriber.cc | 5 +-
be/src/statestore/statestore-subscriber.h | 7 +++
tests/custom_cluster/test_restart_services.py | 68 +++++++++++++++++++++++++++
4 files changed, 91 insertions(+), 2 deletions(-)
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index 863ff16..d584f6f 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -238,6 +238,13 @@ DEFINE_int64(shutdown_deadline_s, 60 * 60, "Default time limit in seconds for th
"down process. If this duration elapses after the shut down process is started, "
"the daemon shuts down regardless of any running queries.");
+DEFINE_int64_hidden(failed_backends_query_cancellation_grace_period_ms, 30000L,
+ "Grace period since last successful subscriber registration that impala server is "
+ "willing to wait before initiating cancellation of queries running on backends not "
+ "included in the latest membership update. This value should be large enough to give "
+ "the statestore enough time to get a consistent view of cluster membership after "
+ "recovery.");
+
#ifndef NDEBUG
DEFINE_int64(stress_metadata_loading_pause_injection_ms, 0, "Simulates metadata loading"
"for a given query by injecting a sleep equivalent to this configuration in "
@@ -1814,7 +1821,11 @@ void ImpalaServer::MembershipCallback(
}
}
- CancelQueriesOnFailedBackends(current_membership);
+ // Only initiate cancellation after a grace period since last successful registration.
+ if (exec_env_->subscriber()->MilliSecondsSinceLastRegistration()
+ >= FLAGS_failed_backends_query_cancellation_grace_period_ms) {
+ CancelQueriesOnFailedBackends(current_membership);
+ }
}
void ImpalaServer::CancelQueriesOnFailedBackends(
diff --git a/be/src/statestore/statestore-subscriber.cc b/be/src/statestore/statestore-subscriber.cc
index c83b520..9fb4c9b 100644
--- a/be/src/statestore/statestore-subscriber.cc
+++ b/be/src/statestore/statestore-subscriber.cc
@@ -182,7 +182,10 @@ Status StatestoreSubscriber::Register() {
RETURN_IF_ERROR(client.DoRpc(&StatestoreServiceClientWrapper::RegisterSubscriber,
request, &response));
Status status = Status(response.status);
- if (status.ok()) connected_to_statestore_metric_->SetValue(true);
+ if (status.ok()) {
+ connected_to_statestore_metric_->SetValue(true);
+ last_registration_ms_.Store(MonotonicMillis());
+ }
if (response.__isset.registration_id) {
lock_guard<mutex> l(registration_id_lock_);
registration_id_ = response.registration_id;
diff --git a/be/src/statestore/statestore-subscriber.h b/be/src/statestore/statestore-subscriber.h
index 016343f..c187f17 100644
--- a/be/src/statestore/statestore-subscriber.h
+++ b/be/src/statestore/statestore-subscriber.h
@@ -127,6 +127,10 @@ class StatestoreSubscriber {
const std::string& id() const { return subscriber_id_; }
+ int64_t MilliSecondsSinceLastRegistration() const {
+ return MonotonicMillis() - last_registration_ms_.Load();
+ }
+
private:
/// Unique, but opaque, identifier for this subscriber.
const std::string subscriber_id_;
@@ -207,6 +211,9 @@ class StatestoreSubscriber {
/// registration.
RegistrationId registration_id_;
+ /// Monotonic timestamp of the last successful registration.
+ AtomicInt64 last_registration_ms_{0};
+
struct TopicRegistration {
/// Held when processing a topic update. 'StatestoreSubscriber::lock_' must be held in
/// shared mode before acquiring this lock. If taking multiple update locks, they must
diff --git a/tests/custom_cluster/test_restart_services.py b/tests/custom_cluster/test_restart_services.py
index ca6ffaf..0591ef2 100644
--- a/tests/custom_cluster/test_restart_services.py
+++ b/tests/custom_cluster/test_restart_services.py
@@ -29,6 +29,7 @@ from time import sleep
from impala.error import HiveServer2Error
from TCLIService import TCLIService
+from beeswaxd.BeeswaxService import QueryState
from tests.beeswax.impala_beeswax import ImpalaBeeswaxException
from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
from tests.common.skip import SkipIfEC
@@ -88,6 +89,73 @@ class TestRestart(CustomClusterTestSuite):
client.close()
+ SUBSCRIBER_TIMEOUT_S = 2
+ CANCELLATION_GRACE_PERIOD_S = 5
+
+ @pytest.mark.execute_serially
+ @CustomClusterTestSuite.with_args(
+ impalad_args="--statestore_subscriber_timeout_seconds={timeout_s} "
+ "--failed_backends_query_cancellation_grace_period_ms={grace_period_ms}"
+ .format(timeout_s=SUBSCRIBER_TIMEOUT_S,
+ grace_period_ms=(CANCELLATION_GRACE_PERIOD_S * 1000)),
+ catalogd_args="--statestore_subscriber_timeout_seconds={timeout_s}".format(
+ timeout_s=SUBSCRIBER_TIMEOUT_S))
+ def test_restart_statestore_query_resilience(self):
+ """IMPALA-7665: Test that after restarting statestore a momentary inconsistent
+ cluster membership state will not result in query cancellation. Also make sure that
+ queries get cancelled if a backend actually went down while the statestore was
+ down or during the grace period."""
+ slow_query = \
+ "select distinct * from tpch_parquet.lineitem where l_orderkey > sleep(1000)"
+ impalad = self.cluster.impalads[0]
+ client = impalad.service.create_beeswax_client()
+ try:
+ handle = client.execute_async(slow_query)
+ # Make sure query starts running.
+ self.wait_for_state(handle, QueryState.RUNNING, 1000)
+ # Restart Statestore and wait till the grace period ends + some buffer.
+ self.cluster.statestored.restart()
+ self.cluster.statestored.service.wait_for_live_subscribers(4)
+ sleep(self.CANCELLATION_GRACE_PERIOD_S + 1)
+ assert client.get_state(handle) == QueryState.RUNNING
+ # Now restart statestore and kill a backend while it is down, and make sure the
+ # query fails when it comes back up.
+ start_time = time.time()
+ self.cluster.statestored.kill()
+ self.cluster.impalads[1].kill()
+ self.cluster.statestored.start()
+ try:
+ client.wait_for_finished_timeout(handle, 100)
+ assert False, "Query expected to fail"
+ except ImpalaBeeswaxException as e:
+ assert "Failed due to unreachable impalad" in str(e), str(e)
+ assert time.time() - start_time > self.CANCELLATION_GRACE_PERIOD_S + \
+ self.SUBSCRIBER_TIMEOUT_S, \
+ "Query got cancelled earlier than the cancellation grace period"
+ # Now restart statestore and kill a backend after it comes back up, and make sure
+ # the query eventually fails.
+ # Make sure the new statestore has received update from catalog and sent it to the
+ # impalad.
+ catalogd_version = self.cluster.catalogd.service.get_catalog_version()
+ impalad.service.wait_for_metric_value("catalog.curr-version", catalogd_version)
+ handle = client.execute_async(slow_query)
+ self.wait_for_state(handle, QueryState.RUNNING, 1000)
+ start_time = time.time()
+ self.cluster.statestored.restart()
+ # Make sure it has connected to the impalads before killing one.
+ self.cluster.statestored.service.wait_for_live_subscribers(3)
+ self.cluster.impalads[2].kill()
+ try:
+ client.wait_for_finished_timeout(handle, 100)
+ assert False, "Query expected to fail"
+ except ImpalaBeeswaxException as e:
+ assert "Failed due to unreachable impalad" in str(e), str(e)
+ assert time.time() - start_time > self.CANCELLATION_GRACE_PERIOD_S + \
+ self.SUBSCRIBER_TIMEOUT_S, \
+ "Query got cancelled earlier than the cancellation grace period"
+ finally:
+ client.close()
+
def parse_shutdown_result(result):
"""Parse the shutdown result string and return the strings (grace left,