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/03/05 23:39:10 UTC

[1/3] impala git commit: IMPALA-6601: fix ASAN issue in row-batch-serialize-test

Repository: impala
Updated Branches:
  refs/heads/2.x 65afb1cd2 -> a73b8f833


IMPALA-6601: fix ASAN issue in row-batch-serialize-test

Constructing a StringValue with a std::string requires
that the std::string remain valid for as long as the
StringValue is valid. row-batch-serialize-test's
TestRowBatchLimits uses a temporary string value that
does not have the lifetime needed. When the memory is
reused, ASAN complains.

This changes TestRowBatchLimits to use strings with an
appropriate lifetime. The test now runs successfully
under ASAN.

Change-Id: I553b004f165be1d8027e3804852f357fa9843f7d
Reviewed-on: http://gerrit.cloudera.org:8080/9488
Reviewed-by: Lars Volker <lv...@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/f1c3e005
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/f1c3e005
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/f1c3e005

Branch: refs/heads/2.x
Commit: f1c3e005a8625719f1e7b7f970db6ca6a6110de7
Parents: 65afb1c
Author: Joe McDonnell <jo...@cloudera.com>
Authored: Sun Mar 4 14:49:05 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Mon Mar 5 03:10:15 2018 +0000

----------------------------------------------------------------------
 be/src/runtime/row-batch-serialize-test.cc | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/f1c3e005/be/src/runtime/row-batch-serialize-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/row-batch-serialize-test.cc b/be/src/runtime/row-batch-serialize-test.cc
index f6a048d..b6a0009 100644
--- a/be/src/runtime/row-batch-serialize-test.cc
+++ b/be/src/runtime/row-batch-serialize-test.cc
@@ -141,17 +141,20 @@ class RowBatchSerializeTest : public testing::Test {
 
     // Write string #1
     SlotDescriptor* string1_desc = tuple_desc->slots()[1];
-    StringValue sv1(string(string1_size, 'a'));
+    string string1(string1_size, 'a');
+    StringValue sv1(string1);
     RawValue::Write(&sv1, tuple, string1_desc, batch->tuple_data_pool());
 
     // Write string #2
     SlotDescriptor* string2_desc = tuple_desc->slots()[2];
-    StringValue sv2(string(string2_size, 'a'));
+    string string2(string2_size, 'a');
+    StringValue sv2(string2);
     RawValue::Write(&sv2, tuple, string2_desc, batch->tuple_data_pool());
 
     // Write string #3
     SlotDescriptor* string3_desc = tuple_desc->slots()[3];
-    StringValue sv3(string(string3_size, 'a'));
+    string string3(string3_size, 'a');
+    StringValue sv3(string3);
     RawValue::Write(&sv3, tuple, string3_desc, batch->tuple_data_pool());
 
     // Done with this row


[2/3] impala git commit: IMPALA-2567: Enable KRPC by default

Posted by ta...@apache.org.
IMPALA-2567: Enable KRPC by default

This change enables the switch to use KRPC by default.
This change also fixes a bug in KrpcDataStreamMgr to
check if maintenance thread was started before calling
Join() on it. This shows up in BE tests as the maintenance
thread isn't started in them.

Testing done: exhaustive build.

Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Reviewed-on: http://gerrit.cloudera.org:8080/9461
Reviewed-by: Michael Ho <kw...@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/5606a4d4
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/5606a4d4
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/5606a4d4

Branch: refs/heads/2.x
Commit: 5606a4d45f4b6604ee54aeceb1815a3b656dddc6
Parents: f1c3e00
Author: Michael Ho <kw...@cloudera.com>
Authored: Tue Feb 20 14:45:47 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Mon Mar 5 19:10:13 2018 +0000

----------------------------------------------------------------------
 be/src/runtime/exec-env.cc                |  5 ++---
 be/src/runtime/krpc-data-stream-mgr.cc    |  2 +-
 bin/run-all-tests.sh                      | 18 +++++++++---------
 bin/start-impala-cluster.py               |  8 ++++----
 tests/common/custom_cluster_test_suite.py |  4 ++--
 tests/common/skip.py                      |  4 ++--
 tests/common/test_skip.py                 |  4 ++--
 tests/conftest.py                         |  6 +++---
 8 files changed, 25 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/5606a4d4/be/src/runtime/exec-env.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/exec-env.cc b/be/src/runtime/exec-env.cc
index 51eece5..66d7f3c 100644
--- a/be/src/runtime/exec-env.cc
+++ b/be/src/runtime/exec-env.cc
@@ -81,9 +81,8 @@ DEFINE_int32(state_store_subscriber_port, 23000,
 DEFINE_int32(num_hdfs_worker_threads, 16,
     "(Advanced) The number of threads in the global HDFS operation pool");
 DEFINE_bool(disable_admission_control, false, "Disables admission control.");
-DEFINE_bool_hidden(use_krpc, false, "Used to indicate whether to use KRPC for the "
-    "DataStream subsystem, or the Thrift RPC layer instead. Defaults to false. "
-    "KRPC not yet supported");
+DEFINE_bool(use_krpc, true, "If true, use KRPC for the DataStream subsystem. "
+    "Otherwise use Thrift RPC.");
 
 DECLARE_int32(state_store_port);
 DECLARE_int32(num_threads_per_core);

http://git-wip-us.apache.org/repos/asf/impala/blob/5606a4d4/be/src/runtime/krpc-data-stream-mgr.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/krpc-data-stream-mgr.cc b/be/src/runtime/krpc-data-stream-mgr.cc
index 40dd285..8005183 100644
--- a/be/src/runtime/krpc-data-stream-mgr.cc
+++ b/be/src/runtime/krpc-data-stream-mgr.cc
@@ -418,7 +418,7 @@ KrpcDataStreamMgr::~KrpcDataStreamMgr() {
   shutdown_promise_.Set(true);
   deserialize_pool_.Shutdown();
   LOG(INFO) << "Waiting for data-stream-mgr maintenance thread...";
-  maintenance_thread_->Join();
+  if (maintenance_thread_.get() != nullptr) maintenance_thread_->Join();
   LOG(INFO) << "Waiting for deserialization thread pool...";
   deserialize_pool_.Join();
 }

http://git-wip-us.apache.org/repos/asf/impala/blob/5606a4d4/bin/run-all-tests.sh
----------------------------------------------------------------------
diff --git a/bin/run-all-tests.sh b/bin/run-all-tests.sh
index 42ae2e3..c5661c3 100755
--- a/bin/run-all-tests.sh
+++ b/bin/run-all-tests.sh
@@ -38,8 +38,8 @@ if "${CLUSTER_DIR}/admin" is_kerberized; then
   KERB_ARGS="--use_kerberos"
 fi
 
-# Enable KRPC during tests (default: false).
-: ${TEST_KRPC:=false}
+# Enable KRPC during tests (default: true).
+: ${TEST_KRPC:=true}
 
 # Parametrized Test Options
 # Run FE Tests
@@ -66,9 +66,9 @@ 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"
+# If KRPC tests are disabled, pass the flag to disable KRPC during cluster start.
+if [[ "${TEST_KRPC}" == "false" ]]; then
+  TEST_START_CLUSTER_ARGS="${TEST_START_CLUSTER_ARGS} --disable_krpc"
 fi
 
 # Indicates whether code coverage reports should be generated.
@@ -119,10 +119,10 @@ 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"
+# If KRPC tests are disabled, pass test_no_krpc flag to pytest.
+# This includes the end-to-end tests and the custom cluster tests.
+if [[ "${TEST_KRPC}" == "false" ]]; then
+  COMMON_PYTEST_ARGS+=" --test_no_krpc"
 fi
 
 # For logging when using run-step.

http://git-wip-us.apache.org/repos/asf/impala/blob/5606a4d4/bin/start-impala-cluster.py
----------------------------------------------------------------------
diff --git a/bin/start-impala-cluster.py b/bin/start-impala-cluster.py
index 248b6d5..a4326f5 100755
--- a/bin/start-impala-cluster.py
+++ b/bin/start-impala-cluster.py
@@ -53,8 +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("--disable_krpc", dest="disable_krpc", action="store_true",
+                  default=False, help="Disable KRPC DataStream service during startup.")
 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.")
@@ -260,8 +260,8 @@ 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)
+    if options.disable_krpc:
+      args = "-use_krpc=false %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/5606a4d4/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 10c94fc..20037bf 100644
--- a/tests/common/custom_cluster_test_suite.py
+++ b/tests/common/custom_cluster_test_suite.py
@@ -137,8 +137,8 @@ class CustomClusterTestSuite(ImpalaTestSuite):
     if use_exclusive_coordinators:
       cmd.append("--use_exclusive_coordinators")
 
-    if pytest.config.option.test_krpc:
-      cmd.append("--use_krpc")
+    if pytest.config.option.test_no_krpc:
+      cmd.append("--disable_krpc")
 
     try:
       check_call(cmd + options, close_fds=True)

http://git-wip-us.apache.org/repos/asf/impala/blob/5606a4d4/tests/common/skip.py
----------------------------------------------------------------------
diff --git a/tests/common/skip.py b/tests/common/skip.py
index f4f3a29..3113acf 100644
--- a/tests/common/skip.py
+++ b/tests/common/skip.py
@@ -85,9 +85,9 @@ 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,
+  not_krpc = pytest.mark.skipif(pytest.config.option.test_no_krpc,
       reason="Test is only supported when using KRPC.")
-  not_thrift = pytest.mark.skipif(pytest.config.option.test_krpc,
+  not_thrift = pytest.mark.skipif(not pytest.config.option.test_no_krpc,
       reason="Test is only supported when using Thrift RPC.")
 
 class SkipIfIsilon:

http://git-wip-us.apache.org/repos/asf/impala/blob/5606a4d4/tests/common/test_skip.py
----------------------------------------------------------------------
diff --git a/tests/common/test_skip.py b/tests/common/test_skip.py
index 60340cd..3e7d281 100644
--- a/tests/common/test_skip.py
+++ b/tests/common/test_skip.py
@@ -32,8 +32,8 @@ class TestSkipIf(ImpalaTestSuite):
 
   @SkipIf.not_krpc
   def test_skip_if_not_krpc(self):
-    assert pytest.config.option.test_krpc
+    assert not pytest.config.option.test_no_krpc
 
   @SkipIf.not_thrift
   def test_skip_if_not_thrift(self):
-    assert not pytest.config.option.test_krpc
+    assert pytest.config.option.test_no_krpc

http://git-wip-us.apache.org/repos/asf/impala/blob/5606a4d4/tests/conftest.py
----------------------------------------------------------------------
diff --git a/tests/conftest.py b/tests/conftest.py
index e60d041..144fc5c 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -116,9 +116,9 @@ 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.")
+  parser.addoption("--test_no_krpc", dest="test_no_krpc", action="store_true",
+                   default=False, help="Run all tests with KRPC disabled. This assumes "
+                   "that the test cluster has been started with --disable_krpc.")
 
 
 def pytest_assertrepr_compare(op, left, right):


[3/3] impala git commit: IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters

Posted by ta...@apache.org.
IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters

Noted that after enabling ha-proxy in a kerberized cluster,
users can no-longer connect to individual impala daemons directly
from impala shell.

Change-Id: I1ce9930e3f658c52502a2ba95f93647e2706d58c
Reviewed-on: http://gerrit.cloudera.org:8080/9286
Reviewed-by: Sailesh Mukil <sa...@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/a73b8f83
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/a73b8f83
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/a73b8f83

Branch: refs/heads/2.x
Commit: a73b8f83314b5b11d47b0af6143ffd2941d932f7
Parents: 5606a4d
Author: Alex Rodoni <ar...@cloudera.com>
Authored: Mon Feb 12 14:16:32 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Mon Mar 5 19:10:13 2018 +0000

----------------------------------------------------------------------
 docs/topics/impala_proxy.xml | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/a73b8f83/docs/topics/impala_proxy.xml
----------------------------------------------------------------------
diff --git a/docs/topics/impala_proxy.xml b/docs/topics/impala_proxy.xml
index cd68d97..6f1c780 100644
--- a/docs/topics/impala_proxy.xml
+++ b/docs/topics/impala_proxy.xml
@@ -237,10 +237,18 @@ under the License.
     <conbody>
 
       <p>
-        In a cluster using Kerberos, applications check host credentials to verify that the host they are
-        connecting to is the same one that is actually processing the request, to prevent man-in-the-middle
-        attacks. To clarify that the load-balancing proxy server is legitimate, perform these extra Kerberos setup
-        steps:
+        In a cluster using Kerberos, applications check host credentials to
+        verify that the host they are connecting to is the same one that is
+        actually processing the request, to prevent man-in-the-middle attacks.
+      </p>
+      <note>
+          Once you enable a proxy server in a Kerberized cluster, users will not
+          be able to connect to individual impala daemons directly from impala
+          shell.
+      </note>
+      <p>
+        To clarify that the load-balancing proxy server is legitimate, perform
+        these extra Kerberos setup steps:
       </p>
 
       <ol>