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 2016/12/02 17:06:50 UTC
[1/2] incubator-impala git commit: IMPALA-4567: Fix
test_kudu_alter_table exhaustive failures
Repository: incubator-impala
Updated Branches:
refs/heads/master da34ce978 -> d44df8368
IMPALA-4567: Fix test_kudu_alter_table exhaustive failures
The issue is that we set the Kudu table name explicitly via
tblproperty so it doesn't have the unique db name in the
underlying Kudu name. Meanwhile, the tests are run
concurrently in exhaustive so this test may end up running
the multiple times (w/ different parameters, e.g.
disable_codegen) concurrently. This test needs to be run
serially.
Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968
Reviewed-on: http://gerrit.cloudera.org:8080/5312
Reviewed-by: David Knupp <dk...@cloudera.com>
Reviewed-by: Dimitris Tsirogiannis <dt...@cloudera.com>
Tested-by: Internal Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/48983b38
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/48983b38
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/48983b38
Branch: refs/heads/master
Commit: 48983b389366eb7f792f5e309fa3eb77b70990f6
Parents: da34ce9
Author: Matthew Jacobs <mj...@cloudera.com>
Authored: Thu Dec 1 12:59:47 2016 -0800
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Fri Dec 2 04:01:19 2016 +0000
----------------------------------------------------------------------
tests/query_test/test_kudu.py | 1 +
1 file changed, 1 insertion(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/48983b38/tests/query_test/test_kudu.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_kudu.py b/tests/query_test/test_kudu.py
index 3401ae2..b376ea9 100644
--- a/tests/query_test/test_kudu.py
+++ b/tests/query_test/test_kudu.py
@@ -58,6 +58,7 @@ class TestKuduOperations(KuduTestSuite):
def test_kudu_partition_ddl(self, vector, unique_database):
self.run_test_case('QueryTest/kudu_partition_ddl', vector, use_db=unique_database)
+ @pytest.mark.execute_serially
def test_kudu_alter_table(self, vector, unique_database):
self.run_test_case('QueryTest/kudu_alter', vector, use_db=unique_database)
[2/2] incubator-impala git commit: IMPALA-4525: follow-on: cleanup
error handling
Posted by ta...@apache.org.
IMPALA-4525: follow-on: cleanup error handling
Testing:
Ran exhaustive build. There is already some test coverage in
test_exprs.py for errors returned during constant expr evaluation by the
frontend.
Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0
Reviewed-on: http://gerrit.cloudera.org:8080/5212
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/d44df836
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/d44df836
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/d44df836
Branch: refs/heads/master
Commit: d44df83689f6cd0cb5c93d3893beaff4f7c9342a
Parents: 48983b3
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Wed Nov 23 15:48:55 2016 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Dec 2 11:37:14 2016 +0000
----------------------------------------------------------------------
be/src/exprs/expr-context.cc | 4 +--
be/src/exprs/expr-context.h | 3 ++
be/src/service/fe-support.cc | 67 +++++++++++++++++----------------------
3 files changed, 34 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d44df836/be/src/exprs/expr-context.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-context.cc b/be/src/exprs/expr-context.cc
index 27c61c1..94c652a 100644
--- a/be/src/exprs/expr-context.cc
+++ b/be/src/exprs/expr-context.cc
@@ -71,9 +71,9 @@ Status ExprContext::Open(RuntimeState* state) {
}
void ExprContext::Close(RuntimeState* state) {
- DCHECK(!closed_);
+ if (closed_) return;
FunctionContext::FunctionStateScope scope =
- is_clone_? FunctionContext::THREAD_LOCAL : FunctionContext::FRAGMENT_LOCAL;
+ is_clone_ ? FunctionContext::THREAD_LOCAL : FunctionContext::FRAGMENT_LOCAL;
root_->Close(state, this, scope);
for (int i = 0; i < fn_contexts_.size(); ++i) {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d44df836/be/src/exprs/expr-context.h
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-context.h b/be/src/exprs/expr-context.h
index e765b87..f7e1239 100644
--- a/be/src/exprs/expr-context.h
+++ b/be/src/exprs/expr-context.h
@@ -49,6 +49,8 @@ class ExprContext {
/// Prepare expr tree for evaluation.
/// Allocations from this context will be counted against 'tracker'.
+ /// If Prepare() is called, Close() must be called before destruction to release
+ /// resources, regardless of whether Prepare() succeeded.
Status Prepare(RuntimeState* state, const RowDescriptor& row_desc,
MemTracker* tracker);
@@ -68,6 +70,7 @@ class ExprContext {
Status Clone(RuntimeState* state, ExprContext** new_context);
/// Closes all FunctionContexts. Must be called on every ExprContext, including clones.
+ /// Has no effect if already closed.
void Close(RuntimeState* state);
/// Calls the appropriate Get*Val() function on this context's expr tree and stores the
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d44df836/be/src/service/fe-support.cc
----------------------------------------------------------------------
diff --git a/be/src/service/fe-support.cc b/be/src/service/fe-support.cc
index 93eff4b..b3e3d0c 100644
--- a/be/src/service/fe-support.cc
+++ b/be/src/service/fe-support.cc
@@ -79,11 +79,13 @@ JNIEXPORT jbyteArray JNICALL
Java_org_apache_impala_service_FeSupport_NativeEvalConstExprs(
JNIEnv* env, jclass caller_class, jbyteArray thrift_expr_batch,
jbyteArray thrift_query_ctx_bytes) {
+ Status status;
jbyteArray result_bytes = NULL;
TQueryCtx query_ctx;
TExprBatch expr_batch;
JniLocalFrame jni_frame;
TResultRow expr_results;
+ vector<TColumnValue> results;
ObjectPool obj_pool;
DeserializeThriftMsg(env, thrift_expr_batch, &expr_batch);
@@ -113,71 +115,60 @@ Java_org_apache_impala_service_FeSupport_NativeEvalConstExprs(
vector<ExprContext*> expr_ctxs;
for (const TExpr& texpr : texprs) {
ExprContext* ctx;
- THROW_IF_ERROR_RET(Expr::CreateExprTree(&obj_pool, texpr, &ctx), env,
- JniUtil::internal_exc_class(), result_bytes);
- Status prepare_status =
- ctx->Prepare(&state, RowDescriptor(), state.query_mem_tracker());
+ status = Expr::CreateExprTree(&obj_pool, texpr, &ctx);
+ if (!status.ok()) goto error;
+
+ // Add 'ctx' to vector so it will be closed if Prepare() fails.
expr_ctxs.push_back(ctx);
- if (!prepare_status.ok()) {
- for (ExprContext* ctx: expr_ctxs) ctx->Close(&state);
- (env)->ThrowNew(JniUtil::internal_exc_class(), prepare_status.GetDetail().c_str());
- return result_bytes;
- }
+ status = ctx->Prepare(&state, RowDescriptor(), state.query_mem_tracker());
+ if (!status.ok()) goto error;
}
// UDFs which cannot be interpreted need to be handled by codegen.
if (state.ScalarFnNeedsCodegen()) {
- THROW_IF_ERROR_RET(
- state.CreateCodegen(), env, JniUtil::internal_exc_class(), result_bytes);
+ status = state.CreateCodegen();
+ if (!status.ok()) goto error;
LlvmCodeGen* codegen = state.codegen();
DCHECK(codegen != NULL);
state.CodegenScalarFns();
codegen->EnableOptimizations(false);
Status status = codegen->FinalizeModule();
- if (!status.ok()) {
- for (int i = 0; i < expr_ctxs.size(); ++i) {
- expr_ctxs[i]->Close(&state);
- }
- (env)->ThrowNew(JniUtil::internal_exc_class(), status.GetDetail().c_str());
- return result_bytes;
- }
+ if (!status.ok()) goto error;
}
// Open() and evaluate the exprs. Always Close() the exprs even in case of errors.
- vector<TColumnValue> results;
- Status status;
- int i;
- for (i = 0; i < expr_ctxs.size(); ++i) {
- status = expr_ctxs[i]->Open(&state);
- if (!status.ok()) break;
+ for (ExprContext* expr_ctx : expr_ctxs) {
+ status = expr_ctx->Open(&state);
+ if (!status.ok()) goto error;
TColumnValue val;
- expr_ctxs[i]->GetConstantValue(&val);
- status = expr_ctxs[i]->root()->GetFnContextError(expr_ctxs[i]);
- if (!status.ok()) break;
+ expr_ctx->GetConstantValue(&val);
+ status = expr_ctx->root()->GetFnContextError(expr_ctx);
+ if (!status.ok()) goto error;
// Check for mem limit exceeded.
status = state.CheckQueryState();
- if (!status.ok()) break;
+ if (!status.ok()) goto error;
// Check for warnings registered in the runtime state.
if (state.HasErrors()) {
status = Status(state.ErrorLog());
- break;
+ goto error;
}
- expr_ctxs[i]->Close(&state);
+ expr_ctx->Close(&state);
results.push_back(val);
}
- // Convert status to exception. Close all remaining expr contexts.
- if (!status.ok()) {
- for (int j = i; j < expr_ctxs.size(); ++j) expr_ctxs[j]->Close(&state);
- (env)->ThrowNew(JniUtil::internal_exc_class(), status.GetDetail().c_str());
- return result_bytes;
- }
expr_results.__set_colVals(results);
- THROW_IF_ERROR_RET(SerializeThriftMsg(env, &expr_results, &result_bytes), env,
- JniUtil::internal_exc_class(), result_bytes);
+ status = SerializeThriftMsg(env, &expr_results, &result_bytes);
+ if (!status.ok()) goto error;
+ return result_bytes;
+
+error:
+ DCHECK(!status.ok());
+ // Convert status to exception. Close all remaining expr contexts.
+ for (ExprContext* expr_ctx : expr_ctxs) expr_ctx->Close(&state);
+ (env)->ThrowNew(JniUtil::internal_exc_class(), status.GetDetail().c_str());
return result_bytes;
}