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 2018/02/17 00:17:27 UTC

[1/4] impala git commit: IMPALA-6508: add KRPC test flag

Repository: impala
Updated Branches:
  refs/heads/master cb05241c8 -> 9c0b1ba33


IMPALA-6508: add KRPC test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable KRPC for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with KRPC support or not.

- SkipIf.not_krpc can be used to mark a test that depends on KRPC.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Reviewed-on: http://gerrit.cloudera.org:8080/9291
Reviewed-by: David Knupp <dk...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/a8fc9f0f
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/a8fc9f0f
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/a8fc9f0f

Branch: refs/heads/master
Commit: a8fc9f0fc7925c106a6a290420d0f6bc42460af4
Parents: cb05241
Author: Lars Volker <lv...@cloudera.com>
Authored: Mon Feb 12 14:48:58 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Feb 16 09:26:01 2018 +0000

----------------------------------------------------------------------
 bin/run-all-tests.sh                        | 18 ++++++++++-
 bin/start-impala-cluster.py                 |  5 +++
 tests/common/custom_cluster_test_suite.py   | 11 ++++++-
 tests/common/skip.py                        |  4 +++
 tests/common/test_skip.py                   | 39 ++++++++++++++++++++++++
 tests/conftest.py                           |  4 +++
 tests/custom_cluster/test_krpc_mem_usage.py |  8 ++---
 tests/custom_cluster/test_rpc_exception.py  |  4 ++-
 8 files changed, 86 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/a8fc9f0f/bin/run-all-tests.sh
----------------------------------------------------------------------
diff --git a/bin/run-all-tests.sh b/bin/run-all-tests.sh
index 692b510..42ae2e3 100755
--- a/bin/run-all-tests.sh
+++ b/bin/run-all-tests.sh
@@ -38,6 +38,9 @@ if "${CLUSTER_DIR}/admin" is_kerberized; then
   KERB_ARGS="--use_kerberos"
 fi
 
+# Enable KRPC during tests (default: false).
+: ${TEST_KRPC:=false}
+
 # Parametrized Test Options
 # Run FE Tests
 : ${FE_TEST:=true}
@@ -50,7 +53,8 @@ fi
 : ${JDBC_TEST:=true}
 # Run Cluster Tests
 : ${CLUSTER_TEST:=true}
-# Extra arguments passed to start-impala-cluster for tests
+# Extra arguments passed to start-impala-cluster for tests. These do not apply to custom
+# cluster tests.
 : ${TEST_START_CLUSTER_ARGS:=}
 if [[ "${TARGET_FILESYSTEM}" == "local" ]]; then
   # TODO: Remove abort_on_config_error flag from here and create-load-data.sh once
@@ -61,6 +65,12 @@ if [[ "${TARGET_FILESYSTEM}" == "local" ]]; then
 else
   TEST_START_CLUSTER_ARGS="${TEST_START_CLUSTER_ARGS} --cluster_size=3"
 fi
+
+# If KRPC tests are enabled, pass the corresponding flag during cluster start.
+if [[ "${TEST_KRPC}" != "false" ]]; then
+  TEST_START_CLUSTER_ARGS="${TEST_START_CLUSTER_ARGS} --use_krpc"
+fi
+
 # Indicates whether code coverage reports should be generated.
 : ${CODE_COVERAGE:=false}
 
@@ -109,6 +119,12 @@ if [[ "${TARGET_FILESYSTEM}" == "local" ]]; then
   COMMON_PYTEST_ARGS+=" --impalad=localhost:21000"
 fi
 
+# If KRPC tests are enabled, pass the corresponding flag to pytest. This includes the
+# end-to-end tests and the custom cluster tests.
+if [[ "${TEST_KRPC}" != "false" ]]; then
+  COMMON_PYTEST_ARGS+=" --test_krpc"
+fi
+
 # For logging when using run-step.
 LOG_DIR="${IMPALA_EE_TEST_LOGS_DIR}"
 

http://git-wip-us.apache.org/repos/asf/impala/blob/a8fc9f0f/bin/start-impala-cluster.py
----------------------------------------------------------------------
diff --git a/bin/start-impala-cluster.py b/bin/start-impala-cluster.py
index 67ca374..248b6d5 100755
--- a/bin/start-impala-cluster.py
+++ b/bin/start-impala-cluster.py
@@ -53,6 +53,8 @@ parser.add_option("--state_store_args", dest="state_store_args", action="append"
 parser.add_option("--catalogd_args", dest="catalogd_args", action="append",
                   type="string", default=[],
                   help="Additional arguments to pass to the Catalog Service at startup")
+parser.add_option("--use_krpc", dest="use_krpc", action="store_true", default=False,
+                  help="Enable KRPC during startup of the test cluster.")
 parser.add_option("--kill", "--kill_only", dest="kill_only", action="store_true",
                   default=False, help="Instead of starting the cluster, just kill all"
                   " the running impalads and the statestored.")
@@ -258,6 +260,9 @@ def start_impalad_instances(cluster_size, num_coordinators, use_exclusive_coordi
     if i < len(delay_list):
       args = "-stress_catalog_init_delay_ms=%s %s" % (delay_list[i], args)
 
+    if options.use_krpc:
+      args = "-use_krpc=true %s" % (args)
+
     stderr_log_file_path = os.path.join(options.log_dir, '%s-error.log' % service_name)
     exec_impala_process(IMPALAD_PATH, args, stderr_log_file_path)
 

http://git-wip-us.apache.org/repos/asf/impala/blob/a8fc9f0f/tests/common/custom_cluster_test_suite.py
----------------------------------------------------------------------
diff --git a/tests/common/custom_cluster_test_suite.py b/tests/common/custom_cluster_test_suite.py
index c264d0e..10c94fc 100644
--- a/tests/common/custom_cluster_test_suite.py
+++ b/tests/common/custom_cluster_test_suite.py
@@ -125,12 +125,21 @@ class CustomClusterTestSuite(ImpalaTestSuite):
       cluster_size=CLUSTER_SIZE, num_coordinators=NUM_COORDINATORS,
       use_exclusive_coordinators=False, log_level=1, expected_num_executors=CLUSTER_SIZE):
     cls.impala_log_dir = log_dir
+    # We ignore TEST_START_CLUSTER_ARGS here. Custom cluster tests specifically test that
+    # certain custom startup arguments work and we want to keep them independent of dev
+    # environments.
     cmd = [os.path.join(IMPALA_HOME, 'bin/start-impala-cluster.py'),
            '--cluster_size=%d' % cluster_size,
            '--num_coordinators=%d' % num_coordinators,
            '--log_dir=%s' % log_dir,
            '--log_level=%s' % log_level]
-    if use_exclusive_coordinators: cmd.append("--use_exclusive_coordinators")
+
+    if use_exclusive_coordinators:
+      cmd.append("--use_exclusive_coordinators")
+
+    if pytest.config.option.test_krpc:
+      cmd.append("--use_krpc")
+
     try:
       check_call(cmd + options, close_fds=True)
     finally:

http://git-wip-us.apache.org/repos/asf/impala/blob/a8fc9f0f/tests/common/skip.py
----------------------------------------------------------------------
diff --git a/tests/common/skip.py b/tests/common/skip.py
index da3dfe4..f4f3a29 100644
--- a/tests/common/skip.py
+++ b/tests/common/skip.py
@@ -85,6 +85,10 @@ class SkipIf:
   not_hdfs = pytest.mark.skipif(not IS_HDFS, reason="HDFS Filesystem needed")
   no_secondary_fs = pytest.mark.skipif(not SECONDARY_FILESYSTEM,
       reason="Secondary filesystem needed")
+  not_krpc = pytest.mark.skipif(not pytest.config.option.test_krpc,
+      reason="Test is only supported when using KRPC.")
+  not_thrift = pytest.mark.skipif(pytest.config.option.test_krpc,
+      reason="Test is only supported when using Thrift RPC.")
 
 class SkipIfIsilon:
   caching = pytest.mark.skipif(IS_ISILON, reason="SET CACHED not implemented for Isilon")

http://git-wip-us.apache.org/repos/asf/impala/blob/a8fc9f0f/tests/common/test_skip.py
----------------------------------------------------------------------
diff --git a/tests/common/test_skip.py b/tests/common/test_skip.py
new file mode 100644
index 0000000..60340cd
--- /dev/null
+++ b/tests/common/test_skip.py
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+
+from tests.common.impala_test_suite import ImpalaTestSuite
+from tests.common.skip import SkipIf
+
+class TestSkipIf(ImpalaTestSuite):
+  """
+  This suite tests the effectiveness of various SkipIf decorators.
+  TODO: Remove this once we have tests that make use of these decorators.
+  """
+
+  @classmethod
+  def get_workload(cls):
+    return 'functional-query'
+
+  @SkipIf.not_krpc
+  def test_skip_if_not_krpc(self):
+    assert pytest.config.option.test_krpc
+
+  @SkipIf.not_thrift
+  def test_skip_if_not_thrift(self):
+    assert not pytest.config.option.test_krpc

http://git-wip-us.apache.org/repos/asf/impala/blob/a8fc9f0f/tests/conftest.py
----------------------------------------------------------------------
diff --git a/tests/conftest.py b/tests/conftest.py
index 351117d..e60d041 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -116,6 +116,10 @@ def pytest_addoption(parser):
                    help=("Indicates that tests are being run against a remote cluster. "
                          "Some tests may be marked to skip or xfail on remote clusters."))
 
+  parser.addoption("--test_krpc", dest="test_krpc", action="store_true", default=False,
+                   help="Run all tests with KRPC enabled. This assumes that the test "
+                        "cluster has been started with --use-krpc.")
+
 
 def pytest_assertrepr_compare(op, left, right):
   """

http://git-wip-us.apache.org/repos/asf/impala/blob/a8fc9f0f/tests/custom_cluster/test_krpc_mem_usage.py
----------------------------------------------------------------------
diff --git a/tests/custom_cluster/test_krpc_mem_usage.py b/tests/custom_cluster/test_krpc_mem_usage.py
index 07efe2f..07d2757 100644
--- a/tests/custom_cluster/test_krpc_mem_usage.py
+++ b/tests/custom_cluster/test_krpc_mem_usage.py
@@ -19,13 +19,14 @@ import pytest
 import time
 from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
 from tests.common.impala_cluster import ImpalaCluster
-from tests.common.skip import SkipIfBuildType
+from tests.common.skip import SkipIf, SkipIfBuildType
 from tests.verifiers.mem_usage_verifier import MemUsageVerifier
 
 DATA_STREAM_MGR_METRIC = "Data Stream Queued RPC Calls"
 DATA_STREAM_SVC_METRIC = "Data Stream Service"
 ALL_METRICS = [ DATA_STREAM_MGR_METRIC, DATA_STREAM_SVC_METRIC ]
 
+@SkipIf.not_krpc
 class TestKrpcMemUsage(CustomClusterTestSuite):
   """Test for memory usage tracking when using KRPC."""
   TEST_QUERY = "select count(c2.string_col) from \
@@ -62,7 +63,6 @@ class TestKrpcMemUsage(CustomClusterTestSuite):
           assert usage["peak"] > 0, metric_name
 
   @pytest.mark.execute_serially
-  @CustomClusterTestSuite.with_args("--use_krpc")
   def test_krpc_unqueued_memory_usage(self, vector):
     """Executes a simple query and checks that the data stream service consumed some
     memory.
@@ -73,7 +73,7 @@ class TestKrpcMemUsage(CustomClusterTestSuite):
 
   @SkipIfBuildType.not_dev_build
   @pytest.mark.execute_serially
-  @CustomClusterTestSuite.with_args("--use_krpc --stress_datastream_recvr_delay_ms=1000")
+  @CustomClusterTestSuite.with_args("--stress_datastream_recvr_delay_ms=1000")
   def test_krpc_deferred_memory_usage(self, vector):
     """Executes a simple query. The cluster is started with delayed receiver creation to
     trigger RPC queueing.
@@ -82,7 +82,7 @@ class TestKrpcMemUsage(CustomClusterTestSuite):
 
   @SkipIfBuildType.not_dev_build
   @pytest.mark.execute_serially
-  @CustomClusterTestSuite.with_args("--use_krpc --stress_datastream_recvr_delay_ms=1000")
+  @CustomClusterTestSuite.with_args("--stress_datastream_recvr_delay_ms=1000")
   def test_krpc_deferred_memory_cancellation(self, vector):
     """Executes a query and cancels it while RPCs are still queued up. This exercises the
     code to flush the deferred RPC queue in the receiver.

http://git-wip-us.apache.org/repos/asf/impala/blob/a8fc9f0f/tests/custom_cluster/test_rpc_exception.py
----------------------------------------------------------------------
diff --git a/tests/custom_cluster/test_rpc_exception.py b/tests/custom_cluster/test_rpc_exception.py
index d05cb3b..2784e88 100644
--- a/tests/custom_cluster/test_rpc_exception.py
+++ b/tests/custom_cluster/test_rpc_exception.py
@@ -18,7 +18,7 @@
 import pytest
 from tests.beeswax.impala_beeswax import ImpalaBeeswaxException
 from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
-from tests.common.skip import SkipIfBuildType
+from tests.common.skip import SkipIf, SkipIfBuildType
 
 @SkipIfBuildType.not_dev_build
 class TestRPCException(CustomClusterTestSuite):
@@ -68,6 +68,7 @@ class TestRPCException(CustomClusterTestSuite):
   def test_rpc_send_timed_out(self, vector):
     self.execute_test_query(None)
 
+  @SkipIf.not_thrift
   @pytest.mark.execute_serially
   @CustomClusterTestSuite.with_args("--fault_injection_rpc_exception_type=4")
   def test_rpc_recv_closed_connection(self, vector):
@@ -93,6 +94,7 @@ class TestRPCException(CustomClusterTestSuite):
   def test_rpc_secure_send_timed_out(self, vector):
     self.execute_test_query(None)
 
+  @SkipIf.not_thrift
   @pytest.mark.execute_serially
   @CustomClusterTestSuite.with_args("--fault_injection_rpc_exception_type=9")
   def test_rpc_secure_recv_closed_connection(self, vector):


[3/4] impala git commit: [DOCS] Typos fixed in Impala Analytic Functions doc

Posted by ta...@apache.org.
[DOCS] Typos fixed in Impala Analytic Functions doc

Change-Id: Iec4a2822f5e066574e64bf025d300e4cde7a7d29
Reviewed-on: http://gerrit.cloudera.org:8080/9347
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Reviewed-by: John Russell <jr...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/e0abffb9
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/e0abffb9
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/e0abffb9

Branch: refs/heads/master
Commit: e0abffb94f64266bb1b3bcda45b791bf5d51af7d
Parents: 0eaab69
Author: Alex Rodoni <ar...@cloudera.com>
Authored: Thu Feb 15 16:16:37 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Feb 16 23:55:33 2018 +0000

----------------------------------------------------------------------
 docs/shared/impala_common.xml             | 2 +-
 docs/topics/impala_analytic_functions.xml | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/e0abffb9/docs/shared/impala_common.xml
----------------------------------------------------------------------
diff --git a/docs/shared/impala_common.xml b/docs/shared/impala_common.xml
index 410fa9c..a052879 100644
--- a/docs/shared/impala_common.xml
+++ b/docs/shared/impala_common.xml
@@ -2602,7 +2602,7 @@ flight_num:           INT32 SNAPPY DO:83456393 FPO:83488603 SZ:10216514/11474301
       <p rev="" id="analytic_partition_pruning_caveat">
         In queries involving both analytic functions and partitioned tables, partition pruning only occurs for columns named in the <codeph>PARTITION BY</codeph>
         clause of the analytic function call. For example, if an analytic function query has a clause such as <codeph>WHERE year=2016</codeph>,
-        the way to make the query prune all other <codeph>YEAR</codeph> partitions is to include <codeph>PARTITION BY year</codeph>in the analytic function call;
+        the way to make the query prune all other <codeph>YEAR</codeph> partitions is to include <codeph>PARTITION BY year</codeph> in the analytic function call;
         for example, <codeph>OVER (PARTITION BY year,<varname>other_columns</varname> <varname>other_analytic_clauses</varname>)</codeph>.
 <!--
         These examples illustrate the technique:

http://git-wip-us.apache.org/repos/asf/impala/blob/e0abffb9/docs/topics/impala_analytic_functions.xml
----------------------------------------------------------------------
diff --git a/docs/topics/impala_analytic_functions.xml b/docs/topics/impala_analytic_functions.xml
index bdaacca..39d4fc5 100644
--- a/docs/topics/impala_analytic_functions.xml
+++ b/docs/topics/impala_analytic_functions.xml
@@ -781,7 +781,7 @@ order by kind, ordering desc, name;
 
       <p>
         Partitioning by the <codeph>X</codeph> column groups all the duplicate numbers together and returns the
-        place each each value within the group; because each value occurs only 1 or 2 times,
+        place each value within the group; because each value occurs only 1 or 2 times,
         <codeph>DENSE_RANK()</codeph> designates each <codeph>X</codeph> value as either first or second within its
         group.
       </p>
@@ -1569,7 +1569,7 @@ insert into animals values ('Fire-breathing dragon', 'Mythical', NULL);
 
       <p>
         Partitioning by the <codeph>X</codeph> column groups all the duplicate numbers together and returns the
-        place each each value within the group; because each value occurs only 1 or 2 times,
+        place each value within the group; because each value occurs only 1 or 2 times,
         <codeph>RANK()</codeph> designates each <codeph>X</codeph> value as either first or second within its
         group.
       </p>


[2/4] impala git commit: IMPALA-6258: Uninitialized tuple pointers in row batch for empty rows

Posted by ta...@apache.org.
IMPALA-6258: Uninitialized tuple pointers in row batch for empty rows

Tuple pointers in the generated row batches may not be initialized
if a tuple has byte size 0. There are some codes which compare these
uninitialized pointers against nullptr so having them uninitialized
may return wrong (and non-deterministic) results, e.g.:
impala::TupleIsNullPredicate::GetBooleanVal()

The following query produces non-deterministic results currently:

SELECT count(v.x) FROM functional.alltypestiny t3 LEFT OUTER JOIN (
  SELECT true AS x FROM functional.alltypestiny t1 LEFT OUTER JOIN
  functional.alltypestiny t2 ON (true)) v ON (v.x = t3.bool_col)
WHERE t3.bool_col = true;

The alltypestiny table has 8 records, 4 records of them has the true
boolean value for bool_col. Therefore, the above query is a fancy
way of saying "8 * 8 * 4", i.e. it should return 256.

The solution is that scanners initialize tuple pointers to a non-null
value when there are no materialized slots. This non-null value is
provided by the new static member Tuple::POISON.

I extended QueryTest/scanners.test with the above query. This test
executes the query against all table formats.

This change has the biggest performance impact on count(*) queries on
large kudu tables. For my quick benchmark I copied tpch_kudu.lineitem
and doubled its data. The resulting table has 12,002,430 rows.

Without this patch 'select count(*) from biglineitem' runs for ~0.12s.
With the patch applied, the overhead is around a dozens of ms. I measured
the query on my desktop PC using a relase build of Impala. On debug builds,
the execution time of the patched version is around 160% of the original
version.

Without this patch:
+--------------+--------+----------+----------+--------+------------+-----------+---------------+---------------------+
| Operator     | #Hosts | Avg Time | Max Time | #Rows  | Est. #Rows | Peak Mem  | Est. Peak Mem | Detail              |
+--------------+--------+----------+----------+--------+------------+-----------+---------------+---------------------+
| 03:AGGREGATE | 1      | 127.50us | 127.50us | 1      | 1          | 28.00 KB  | 10.00 MB      | FINALIZE            |
| 02:EXCHANGE  | 1      | 22.32ms  | 22.32ms  | 3      | 1          | 0 B       | 0 B           | UNPARTITIONED       |
| 01:AGGREGATE | 3      | 1.78ms   | 1.89ms   | 3      | 1          | 16.00 KB  | 10.00 MB      |                     |
| 00:SCAN KUDU | 3      | 8.00ms   | 8.28ms   | 12.00M | -1         | 512.00 KB | 0 B           | default.biglineitem |
+--------------+--------+----------+----------+--------+------------+-----------+---------------+---------------------+

With this patch:
+--------------+--------+----------+----------+--------+------------+-----------+---------------+---------------------+
| Operator     | #Hosts | Avg Time | Max Time | #Rows  | Est. #Rows | Peak Mem  | Est. Peak Mem | Detail              |
+--------------+--------+----------+----------+--------+------------+-----------+---------------+---------------------+
| 03:AGGREGATE | 1      | 129.01us | 129.01us | 1      | 1          | 28.00 KB  | 10.00 MB      | FINALIZE            |
| 02:EXCHANGE  | 1      | 33.00ms  | 33.00ms  | 3      | 1          | 0 B       | 0 B           | UNPARTITIONED       |
| 01:AGGREGATE | 3      | 1.99ms   | 2.13ms   | 3      | 1          | 16.00 KB  | 10.00 MB      |                     |
| 00:SCAN KUDU | 3      | 13.13ms  | 13.97ms  | 12.00M | -1         | 512.00 KB | 0 B           | default.biglineitem |
+--------------+--------+----------+----------+--------+------------+-----------+---------------+---------------------+

Change-Id: I298122aaaa7e62eb5971508e0698e189519755de
Reviewed-on: http://gerrit.cloudera.org:8080/9239
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/0eaab69f
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/0eaab69f
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/0eaab69f

Branch: refs/heads/master
Commit: 0eaab69fff82a62fbddaae8a0d4ee7a4302ee715
Parents: a8fc9f0
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
Authored: Wed Feb 7 17:05:00 2018 +0100
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Feb 16 20:23:21 2018 +0000

----------------------------------------------------------------------
 be/src/exec/hdfs-scanner.cc                             |  4 +++-
 be/src/exec/kudu-scanner.cc                             |  7 ++++++-
 be/src/runtime/tuple.cc                                 |  2 ++
 be/src/runtime/tuple.h                                  |  4 ++++
 .../functional-query/queries/QueryTest/scanners.test    | 12 ++++++++++++
 5 files changed, 27 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/0eaab69f/be/src/exec/hdfs-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-scanner.cc b/be/src/exec/hdfs-scanner.cc
index f934f79..d62ccc7 100644
--- a/be/src/exec/hdfs-scanner.cc
+++ b/be/src/exec/hdfs-scanner.cc
@@ -220,11 +220,13 @@ int HdfsScanner::WriteTemplateTuples(TupleRow* row, int num_tuples) {
       if (EvalConjuncts(&template_tuple_row)) ++num_to_commit;
     }
   }
+  Tuple** row_tuple = reinterpret_cast<Tuple**>(row);
   if (template_tuple_ != nullptr) {
-    Tuple** row_tuple = reinterpret_cast<Tuple**>(row);
     for (int i = 0; i < num_to_commit; ++i) row_tuple[i] = template_tuple_;
   } else {
     DCHECK_EQ(scan_node_->tuple_desc()->byte_size(), 0);
+    // IMPALA-6258: Initialize tuple ptrs to non-null value
+    for (int i = 0; i < num_to_commit; ++i) row_tuple[i] = Tuple::POISON;
   }
   return num_to_commit;
 }

http://git-wip-us.apache.org/repos/asf/impala/blob/0eaab69f/be/src/exec/kudu-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-scanner.cc b/be/src/exec/kudu-scanner.cc
index a9b56fe..909567c 100644
--- a/be/src/exec/kudu-scanner.cc
+++ b/be/src/exec/kudu-scanner.cc
@@ -255,8 +255,13 @@ Status KuduScanner::HandleEmptyProjection(RowBatch* row_batch) {
       }
     }
   }
+  for (int i = 0; i < num_to_commit; ++i) {
+    // IMPALA-6258: Initialize tuple ptrs to non-null value
+    TupleRow* row = row_batch->GetRow(row_batch->AddRow());
+    row->SetTuple(0, Tuple::POISON);
+    row_batch->CommitLastRow();
+  }
   cur_kudu_batch_num_read_ += rows_to_add;
-  row_batch->CommitRows(num_to_commit);
   return Status::OK();
 }
 

http://git-wip-us.apache.org/repos/asf/impala/blob/0eaab69f/be/src/runtime/tuple.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/tuple.cc b/be/src/runtime/tuple.cc
index cab7686..787c3a4 100644
--- a/be/src/runtime/tuple.cc
+++ b/be/src/runtime/tuple.cc
@@ -46,6 +46,8 @@ const char* SlotOffsets::LLVM_CLASS_NAME = "struct.impala::SlotOffsets";
 const char* Tuple::MATERIALIZE_EXPRS_SYMBOL = "MaterializeExprsILb0ELb0";
 const char* Tuple::MATERIALIZE_EXPRS_NULL_POOL_SYMBOL = "MaterializeExprsILb0ELb1";
 
+Tuple* const Tuple::POISON = reinterpret_cast<Tuple*>(42L);
+
 int64_t Tuple::TotalByteSize(const TupleDescriptor& desc) const {
   int64_t result = desc.byte_size();
   if (!desc.HasVarlenSlots()) return result;

http://git-wip-us.apache.org/repos/asf/impala/blob/0eaab69f/be/src/runtime/tuple.h
----------------------------------------------------------------------
diff --git a/be/src/runtime/tuple.h b/be/src/runtime/tuple.h
index 3286f10..ae9be32 100644
--- a/be/src/runtime/tuple.h
+++ b/be/src/runtime/tuple.h
@@ -84,6 +84,10 @@ class Tuple {
     return result;
   }
 
+  /// Pointer that marks an invalid Tuple address. Rather than leaving Tuple
+  /// pointers uninitialized, they should point to the value of POISON.
+  static Tuple* const POISON;
+
   void Init(int size) { memset(this, 0, size); }
 
   void ClearNullBits(const TupleDescriptor& tuple_desc) {

http://git-wip-us.apache.org/repos/asf/impala/blob/0eaab69f/testdata/workloads/functional-query/queries/QueryTest/scanners.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/scanners.test b/testdata/workloads/functional-query/queries/QueryTest/scanners.test
index 99ec5c5..bd5f496 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/scanners.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/scanners.test
@@ -98,3 +98,15 @@ select count(*) from alltypes where rand() - year > month;
 ---- TYPES
 BIGINT
 ====
+---- QUERY
+# IMPALA-6258: Uninitialized tuple pointers in row batch for empty rows
+# The following query was non-deterministic because of this bug
+select count(v.x) from alltypestiny t3 left outer join (
+  select true as x from alltypestiny t1 left outer join
+  alltypestiny t2 on (true)) v
+on (v.x = t3.bool_col) where t3.bool_col = true
+---- RESULTS
+256
+---- TYPES
+BIGINT
+====


[4/4] impala git commit: [DOCS] Fix in REPLICA_PREFERENCE numeric options

Posted by ta...@apache.org.
[DOCS] Fix in REPLICA_PREFERENCE numeric options

Change-Id: Ia10e69ac38229e0969db11b7edbcf08c2444602b
Reviewed-on: http://gerrit.cloudera.org:8080/9341
Reviewed-by: John Russell <jr...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/9c0b1ba3
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/9c0b1ba3
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/9c0b1ba3

Branch: refs/heads/master
Commit: 9c0b1ba33ccd547ebe63039ba20a0a0c0c87857d
Parents: e0abffb
Author: Alex Rodoni <ar...@cloudera.com>
Authored: Thu Feb 15 14:18:35 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Feb 16 23:56:55 2018 +0000

----------------------------------------------------------------------
 docs/topics/impala_replica_preference.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/9c0b1ba3/docs/topics/impala_replica_preference.xml
----------------------------------------------------------------------
diff --git a/docs/topics/impala_replica_preference.xml b/docs/topics/impala_replica_preference.xml
index 610bd35..45a5dbd 100644
--- a/docs/topics/impala_replica_preference.xml
+++ b/docs/topics/impala_replica_preference.xml
@@ -45,7 +45,7 @@ under the License.
     </p>
 
     <p>
-      <b>Type:</b> numeric (0, 3, 5)
+      <b>Type:</b> numeric (0, 2, 4)
       or corresponding mnemonic strings (<codeph>CACHE_LOCAL</codeph>, <codeph>DISK_LOCAL</codeph>, <codeph>REMOTE</codeph>).
       The gaps in the numeric sequence are to accomodate other intermediate
       values that might be added in the future.