[2/3] incubator-impala git commit: IMPALA-4097: Crash in kudu-scan-node-test

[ab...@apache.org]

IMPALA-4097: Crash in kudu-scan-node-test

The kudu-scan-node-test was calling GetNext() with a NULL
row batch, which isn't valid. This wasn't failing until a
recent code change, and only occasionally due to the timing
of scanner threads producing row batches. One batch is empty
and the other has 1 row. This test worked when the empty row
batch was added to the batch queue first (which usually
happened), but the test code couldn't handle the other
ordering properly. This fixes the test to be more careful
about what is being exercised.

Change-Id: I0e30964232ed137fce7a99bf7e9557293b5905c8
Reviewed-on: http://gerrit.cloudera.org:8080/4337
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/7b4a6fac
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/7b4a6fac
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/7b4a6fac

Branch: refs/heads/master
Commit: 7b4a6fac1acf16fb84b38a005f69c228620ddb3f
Parents: d153d18
Author: Matthew Jacobs <mj...@cloudera.com>
Authored: Thu Sep 8 13:19:47 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Sat Sep 10 00:50:31 2016 +0000

----------------------------------------------------------------------
 be/src/exec/kudu-scan-node-test.cc | 27 ++++++++++++++++++++-------
 be/src/exec/kudu-scan-node.cc      |  1 +
 2 files changed, 21 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/7b4a6fac/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 972ae80..8324628 100644
--- a/be/src/exec/kudu-scan-node-test.cc
+++ b/be/src/exec/kudu-scan-node-test.cc
@@ -312,19 +312,32 @@ TEST_F(KuduScanNodeTest, TestScanEmptyString) {
   vector<TScanRangeParams> params;
   CreateScanRangeParams(num_cols_to_materialize, &params);
   SetUpScanner(&scanner, params);
-  bool eos = false;
   RowBatch* batch = obj_pool_->Add(
       new RowBatch(scanner.row_desc(), DEFAULT_ROWS_PER_BATCH, mem_tracker_.get()));
-  for (int i = 0; i < 2 && batch->num_rows() == 0; ++i) {
-    // Allow for up to 2 empty row batches since there are scanners created for all
-    // tablets (1 split point), and only one row was inserted.
+  bool eos = false;
+  int total_num_rows = 0;
+
+  // Need to call GetNext() a total of 3 times to allow for:
+  // 1) the row batch containing the single row
+  // 2) an empty row batch (since there are scanners created for both tablets)
+  // 3) a final call which returns eos.
+  // The first two may occur in any order and are checked in the for loop below.
+  for (int i = 0; i < 2; ++i) {
     ASSERT_OK(scanner.GetNext(runtime_state_.get(), batch, &eos));
+    ASSERT_FALSE(eos);
+    if (batch->num_rows() > 0) {
+      total_num_rows += batch->num_rows();
+      ASSERT_EQ(PrintBatch(batch), "[(10 null )]\n");
+    }
+    batch->Reset();
   }
-  ASSERT_EQ(1, batch->num_rows());
+  ASSERT_EQ(1, total_num_rows);
 
-  ASSERT_OK(scanner.GetNext(runtime_state_.get(), NULL, &eos));
+  // One last call to find the batch queue empty and GetNext() returns eos.
+  ASSERT_OK(scanner.GetNext(runtime_state_.get(), batch, &eos));
   ASSERT_TRUE(eos);
-  ASSERT_EQ(PrintBatch(batch), "[(10 null )]\n");
+  ASSERT_EQ(0, batch->num_rows());
+
   scanner.Close(runtime_state_.get());
 }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/7b4a6fac/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 b247c67..e171afa 100644
--- a/be/src/exec/kudu-scan-node.cc
+++ b/be/src/exec/kudu-scan-node.cc
@@ -148,6 +148,7 @@ Status KuduScanNode::Open(RuntimeState* state) {
 }
 
 Status KuduScanNode::GetNext(RuntimeState* state, RowBatch* row_batch, bool* eos) {
+  DCHECK(row_batch != NULL);
   RETURN_IF_ERROR(ExecDebugAction(TExecNodePhase::GETNEXT, state));
   RETURN_IF_CANCELLED(state);
   RETURN_IF_ERROR(QueryMaintenance(state));