You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mj...@apache.org on 2016/08/15 23:56:45 UTC
[3/3] incubator-impala git commit: IMPALA-3953: Fixes for
KuduScanNode BE test failure
IMPALA-3953: Fixes for KuduScanNode BE test failure
After a previous fix for IMPALA-3857, KuduScanNodeTest
TestLimitsAreEnforced (BE test) occasionally throws when a
scanner thread takes a lock_ that isn't valid, crashing the
process.
It looks like the issue is likely that TestScanEmptyString
isn't closing its KuduScanNode, and a lingering
ScannerThread may end up touching invalid memory later.
This fixes the test case and also:
1) Adds some missing synchronization in KuduScanNode which
was found in the process of investigating this issue.
2) Adds a DCHECK on ~KuduScanNode() to ensure it was closed.
This was tested by running KuduScanNodeTest in a loop for 5
hours. Without the fix, the failure was produced within
several hours.
Change-Id: I16be206c60a692d2a26d719de8cc45e859b06e97
Reviewed-on: http://gerrit.cloudera.org:8080/3888
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Reviewed-by: Dan Hecht <dh...@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/6fc399eb
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/6fc399eb
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/6fc399eb
Branch: refs/heads/master
Commit: 6fc399ebc435121cdb7865ff4987aca1c95af5fc
Parents: f4da925
Author: Matthew Jacobs <mj...@cloudera.com>
Authored: Wed Aug 10 13:35:52 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Mon Aug 15 23:41:08 2016 +0000
----------------------------------------------------------------------
be/src/exec/kudu-scan-node-test.cc | 1 +
be/src/exec/kudu-scan-node.cc | 3 +++
2 files changed, 4 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/6fc399eb/be/src/exec/kudu-scan-node-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-scan-node-test.cc b/be/src/exec/kudu-scan-node-test.cc
index 7719a75..0cda0ec 100644
--- a/be/src/exec/kudu-scan-node-test.cc
+++ b/be/src/exec/kudu-scan-node-test.cc
@@ -499,6 +499,7 @@ TEST_F(KuduScanNodeTest, TestScanEmptyString) {
ASSERT_OK(scanner.GetNext(runtime_state_.get(), NULL, &eos));
ASSERT_TRUE(eos);
ASSERT_EQ(PrintBatch(batch), "[(10 null )]\n");
+ scanner.Close(runtime_state_.get());
}
// Test that scan limits are enforced.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/6fc399eb/be/src/exec/kudu-scan-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-scan-node.cc b/be/src/exec/kudu-scan-node.cc
index 74011b1..a7a068c 100644
--- a/be/src/exec/kudu-scan-node.cc
+++ b/be/src/exec/kudu-scan-node.cc
@@ -82,6 +82,7 @@ KuduScanNode::KuduScanNode(ObjectPool* pool, const TPlanNode& tnode,
}
KuduScanNode::~KuduScanNode() {
+ DCHECK(is_closed());
STLDeleteElements(&kudu_predicates_);
}
@@ -184,6 +185,8 @@ Status KuduScanNode::GetNext(RuntimeState* state, RowBatch* row_batch, bool* eos
num_rows_returned_ -= num_rows_over;
COUNTER_SET(rows_returned_counter_, num_rows_returned_);
*eos = true;
+
+ unique_lock<mutex> l(lock_);
done_ = true;
materialized_row_batches_->Shutdown();
}