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:22 UTC

impala git commit: IMPALA-6508: add KRPC test flag

Repository: impala
Updated Branches:
  refs/heads/2.x 937d5b21a -> a4bdded6e


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/a4bdded6
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/a4bdded6
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/a4bdded6

Branch: refs/heads/2.x
Commit: a4bdded6e9102907d6cb4bf766cba80b73344c79
Parents: 937d5b2
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 10:53:55 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/a4bdded6/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/a4bdded6/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/a4bdded6/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/a4bdded6/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/a4bdded6/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/a4bdded6/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/a4bdded6/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/a4bdded6/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):