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/09 16:59:29 UTC

[1/6] impala git commit: IMPALA-6075: Add Impala daemon metric for catalog version.

Repository: impala
Updated Branches:
  refs/heads/master 36e301db4 -> 315142190


IMPALA-6075: Add Impala daemon metric for catalog version.

This patch adds new metrics for current version of catalog, current catalog topic version
and catalog service id which are currently used by impala daemon.

Testing:
Verified manually that the new metrics for catalog version, catalog topic version,
catalog service id are displayed and they corresponds to the latest version in
catalogd.INFO log file.

Change-Id: Id2307eb434561ed74ff058106541c0ebda017d97
Reviewed-on: http://gerrit.cloudera.org:8080/8949
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/572af614
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/572af614
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/572af614

Branch: refs/heads/master
Commit: 572af614d6d2af21dc4d261a3c53ed9b2b05c833
Parents: 36e301d
Author: Pranay <ps...@cloudera.com>
Authored: Fri Jan 5 09:31:02 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Wed Feb 7 23:03:40 2018 +0000

----------------------------------------------------------------------
 be/src/service/impala-server.cc |  8 ++++++++
 be/src/service/impala-server.h  |  4 +++-
 be/src/util/impalad-metrics.cc  |  9 +++++++++
 be/src/util/impalad-metrics.h   | 12 ++++++++++++
 common/thrift/metrics.json      | 30 ++++++++++++++++++++++++++++++
 5 files changed, 62 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/572af614/be/src/service/impala-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index cf5f5fb..ee8405b 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -1317,6 +1317,13 @@ Status ImpalaServer::AuthorizeProxyUser(const string& user, const string& do_as_
   return Status(error_msg.str());
 }
 
+void ImpalaServer::CatalogUpdateVersionInfo::UpdateCatalogVersionMetrics()
+{
+  ImpaladMetrics::CATALOG_VERSION->SetValue(catalog_version);
+  ImpaladMetrics::CATALOG_TOPIC_VERSION->SetValue(catalog_topic_version);
+  ImpaladMetrics::CATALOG_SERVICE_ID->SetValue(PrintId(catalog_service_id));
+}
+
 void ImpalaServer::CatalogUpdateCallback(
     const StatestoreSubscriber::TopicDeltaMap& incoming_topic_deltas,
     vector<TTopicDelta>* subscriber_topic_updates) {
@@ -1352,6 +1359,7 @@ void ImpalaServer::CatalogUpdateCallback(
       LOG(INFO) << "Catalog topic update applied with version: " <<
           resp.new_catalog_version << " new min catalog object version: " <<
           resp.min_catalog_object_version;
+      catalog_update_info_.UpdateCatalogVersionMetrics();
     }
     ImpaladMetrics::CATALOG_READY->SetValue(resp.new_catalog_version > 0);
     // TODO: deal with an error status

http://git-wip-us.apache.org/repos/asf/impala/blob/572af614/be/src/service/impala-server.h
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.h b/be/src/service/impala-server.h
index dad1ebf..237b0cb 100644
--- a/be/src/service/impala-server.h
+++ b/be/src/service/impala-server.h
@@ -997,7 +997,9 @@ class ImpalaServer : public ImpalaServiceIf,
       catalog_topic_version(0L),
       min_catalog_object_version(0L) {
     }
-
+    /// Update the metrics to store the current version of catalog, current topic and
+    /// current service id used by impalad.
+    void  UpdateCatalogVersionMetrics();
     /// The last catalog version returned from UpdateCatalog()
     int64_t catalog_version;
     /// The CatalogService ID that this catalog version is from.

http://git-wip-us.apache.org/repos/asf/impala/blob/572af614/be/src/util/impalad-metrics.cc
----------------------------------------------------------------------
diff --git a/be/src/util/impalad-metrics.cc b/be/src/util/impalad-metrics.cc
index 18e96a8..8f5f1be 100644
--- a/be/src/util/impalad-metrics.cc
+++ b/be/src/util/impalad-metrics.cc
@@ -76,6 +76,9 @@ const char* ImpaladMetricKeys::CATALOG_NUM_DBS =
     "catalog.num-databases";
 const char* ImpaladMetricKeys::CATALOG_NUM_TABLES =
     "catalog.num-tables";
+const char* ImpaladMetricKeys::CATALOG_VERSION = "catalog.curr-version";
+const char* ImpaladMetricKeys::CATALOG_TOPIC_VERSION = "catalog.curr-topic";
+const char* ImpaladMetricKeys::CATALOG_SERVICE_ID = "catalog.curr-serviceid";
 const char* ImpaladMetricKeys::CATALOG_READY =
     "catalog.ready";
 const char* ImpaladMetricKeys::NUM_FILES_OPEN_FOR_INSERT =
@@ -126,6 +129,8 @@ IntCounter* ImpaladMetrics::HEDGED_READ_OPS_WIN = NULL;
 // Gauges
 IntGauge* ImpaladMetrics::CATALOG_NUM_DBS = NULL;
 IntGauge* ImpaladMetrics::CATALOG_NUM_TABLES = NULL;
+IntGauge* ImpaladMetrics::CATALOG_VERSION = NULL;
+IntGauge* ImpaladMetrics::CATALOG_TOPIC_VERSION = NULL;
 IntGauge* ImpaladMetrics::IMPALA_SERVER_NUM_OPEN_BEESWAX_SESSIONS = NULL;
 IntGauge* ImpaladMetrics::IMPALA_SERVER_NUM_OPEN_HS2_SESSIONS = NULL;
 IntGauge* ImpaladMetrics::IO_MGR_NUM_BUFFERS = NULL;
@@ -146,6 +151,7 @@ IntGauge* ImpaladMetrics::RESULTSET_CACHE_TOTAL_BYTES = NULL;
 BooleanProperty* ImpaladMetrics::CATALOG_READY = NULL;
 BooleanProperty* ImpaladMetrics::IMPALA_SERVER_READY = NULL;
 StringProperty* ImpaladMetrics::IMPALA_SERVER_VERSION = NULL;
+StringProperty* ImpaladMetrics::CATALOG_SERVICE_ID = NULL;
 
 // Histograms
 HistogramMetric* ImpaladMetrics::QUERY_DURATIONS = NULL;
@@ -235,6 +241,9 @@ void ImpaladMetrics::CreateMetrics(MetricGroup* m) {
   // Initialize catalog metrics
   CATALOG_NUM_DBS = m->AddGauge(ImpaladMetricKeys::CATALOG_NUM_DBS, 0);
   CATALOG_NUM_TABLES = m->AddGauge(ImpaladMetricKeys::CATALOG_NUM_TABLES, 0);
+  CATALOG_VERSION = m->AddGauge(ImpaladMetricKeys::CATALOG_VERSION, 0);
+  CATALOG_TOPIC_VERSION = m->AddGauge(ImpaladMetricKeys::CATALOG_TOPIC_VERSION, 0);
+  CATALOG_SERVICE_ID = m->AddProperty<string>(ImpaladMetricKeys::CATALOG_SERVICE_ID, "");
   CATALOG_READY = m->AddProperty<bool>(ImpaladMetricKeys::CATALOG_READY, false);
 
   // Maximum duration to be tracked by the query durations metric. No particular reasoning

http://git-wip-us.apache.org/repos/asf/impala/blob/572af614/be/src/util/impalad-metrics.h
----------------------------------------------------------------------
diff --git a/be/src/util/impalad-metrics.h b/be/src/util/impalad-metrics.h
index 65ddf1e..f32e3fa 100644
--- a/be/src/util/impalad-metrics.h
+++ b/be/src/util/impalad-metrics.h
@@ -109,6 +109,15 @@ class ImpaladMetricKeys {
   /// Number of DBs in the catalog
   static const char* CATALOG_NUM_DBS;
 
+  /// Current version of catalog with impalad.
+  static const char* CATALOG_VERSION;
+
+  /// Catalog topic version  with impalad.
+  static const char* CATALOG_TOPIC_VERSION;
+
+  /// ServiceID of Catalog with impalad.
+  static const char* CATALOG_SERVICE_ID;
+
   /// Number of tables in the catalog
   static const char* CATALOG_NUM_TABLES;
 
@@ -180,6 +189,8 @@ class ImpaladMetrics {
   // Gauges
   static IntGauge* CATALOG_NUM_DBS;
   static IntGauge* CATALOG_NUM_TABLES;
+  static IntGauge* CATALOG_VERSION;
+  static IntGauge* CATALOG_TOPIC_VERSION;
   static IntGauge* IMPALA_SERVER_NUM_OPEN_BEESWAX_SESSIONS;
   static IntGauge* IMPALA_SERVER_NUM_OPEN_HS2_SESSIONS;
   static IntGauge* IO_MGR_NUM_BUFFERS;
@@ -199,6 +210,7 @@ class ImpaladMetrics {
   static BooleanProperty* CATALOG_READY;
   static BooleanProperty* IMPALA_SERVER_READY;
   static StringProperty* IMPALA_SERVER_VERSION;
+  static StringProperty* CATALOG_SERVICE_ID;
   // Histograms
   static HistogramMetric* QUERY_DURATIONS;
   static HistogramMetric* DDL_DURATIONS;

http://git-wip-us.apache.org/repos/asf/impala/blob/572af614/common/thrift/metrics.json
----------------------------------------------------------------------
diff --git a/common/thrift/metrics.json b/common/thrift/metrics.json
index f493d33..df78a0b 100644
--- a/common/thrift/metrics.json
+++ b/common/thrift/metrics.json
@@ -230,6 +230,36 @@
     "key": "catalog.num-tables"
   },
   {
+    "description": "Catalog topic update version.",
+    "contexts": [
+      "IMPALAD"
+    ],
+    "label": "Catalog topic update version",
+    "units": "NONE",
+    "kind": "GAUGE",
+    "key": "catalog.curr-version"
+  },
+  {
+    "description": "Statestore topic update version.",
+    "contexts": [
+      "IMPALAD"
+    ],
+    "label": "Statestore topic update version",
+    "units": "NONE",
+    "kind": "GAUGE",
+    "key": "catalog.curr-topic"
+  },
+  {
+    "description": "Catalog service id.",
+    "contexts": [
+      "IMPALAD"
+    ],
+    "label": "Catalog service id",
+    "units": "NONE",
+    "kind": "PROPERTY",
+    "key": "catalog.curr-serviceid"
+  },
+  {
     "description": "Indicates if the catalog is ready.",
     "contexts": [
       "IMPALAD"


[5/6] impala git commit: IMPALA-6219: Use AES-GCM for spill-to-disk encryption

Posted by ta...@apache.org.
IMPALA-6219: Use AES-GCM for spill-to-disk encryption

AES-GCM can be very fast(~10 times faster than CFB+SHA256), but it
requires an instruction that Impala can currently run without (CLMUL).
In order to be fast, we dispatch to GCM mode at run-time based on the
CPU and OpenSSL version.

Testing:
run runtime tmp-file-mgr-test, openssl-util-test, buffer-pool-test
and buffered-tuple-stream-test.
add two cases GcmIntegrity & EncryptoArbitraryLength for
openssl-util-test

Change-Id: Ia188a0c5b74e4a22fb30f8c12f65e0469eb75f6b
Reviewed-on: http://gerrit.cloudera.org:8080/9238
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Tim Armstrong <ta...@cloudera.com>


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

Branch: refs/heads/master
Commit: 5aa28b1b3a731a37afa3f028e4f5a39207294ffc
Parents: 9233a8e
Author: Xianda Ke <ke...@gmail.com>
Authored: Wed Feb 7 23:23:00 2018 +0800
Committer: Tim Armstrong <ta...@cloudera.com>
Committed: Fri Feb 9 16:24:00 2018 +0000

----------------------------------------------------------------------
 be/src/runtime/tmp-file-mgr.cc   |  15 +++--
 be/src/util/cpu-info.cc          |  13 +++--
 be/src/util/cpu-info.h           |  13 +++--
 be/src/util/openssl-util-test.cc |  95 ++++++++++++++++++++----------
 be/src/util/openssl-util.cc      | 106 ++++++++++++++++++++++++++++++----
 be/src/util/openssl-util.h       |  70 +++++++++++++++-------
 6 files changed, 233 insertions(+), 79 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/5aa28b1b/be/src/runtime/tmp-file-mgr.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/tmp-file-mgr.cc b/be/src/runtime/tmp-file-mgr.cc
index d35d302..3807670 100644
--- a/be/src/runtime/tmp-file-mgr.cc
+++ b/be/src/runtime/tmp-file-mgr.cc
@@ -612,19 +612,26 @@ void TmpFileMgr::WriteHandle::WaitForWrite() {
 Status TmpFileMgr::WriteHandle::EncryptAndHash(MemRange buffer) {
   DCHECK(FLAGS_disk_spill_encryption);
   SCOPED_TIMER(encryption_timer_);
-  // Since we're using AES-CTR/AES-CFB mode, we must take care not to reuse a
+  // Since we're using GCM/CTR/CFB mode, we must take care not to reuse a
   // key/IV pair. Regenerate a new key and IV for every data buffer we write.
   key_.InitializeRandom();
   RETURN_IF_ERROR(key_.Encrypt(buffer.data(), buffer.len(), buffer.data()));
-  hash_.Compute(buffer.data(), buffer.len());
+
+  if (!key_.IsGcmMode()) {
+    hash_.Compute(buffer.data(), buffer.len());
+  }
   return Status::OK();
 }
 
 Status TmpFileMgr::WriteHandle::CheckHashAndDecrypt(MemRange buffer) {
   DCHECK(FLAGS_disk_spill_encryption);
   SCOPED_TIMER(encryption_timer_);
-  if (!hash_.Verify(buffer.data(), buffer.len())) {
-    return Status("Block verification failure");
+
+  // GCM mode will verify the integrity by itself
+  if (!key_.IsGcmMode()) {
+    if (!hash_.Verify(buffer.data(), buffer.len())) {
+      return Status("Block verification failure");
+    }
   }
   return key_.Decrypt(buffer.data(), buffer.len(), buffer.data());
 }

http://git-wip-us.apache.org/repos/asf/impala/blob/5aa28b1b/be/src/util/cpu-info.cc
----------------------------------------------------------------------
diff --git a/be/src/util/cpu-info.cc b/be/src/util/cpu-info.cc
index a32571e..1e3fcde 100644
--- a/be/src/util/cpu-info.cc
+++ b/be/src/util/cpu-info.cc
@@ -85,12 +85,13 @@ static struct {
   int64_t flag;
 } flag_mappings[] =
 {
-  { "ssse3",  CpuInfo::SSSE3 },
-  { "sse4_1", CpuInfo::SSE4_1 },
-  { "sse4_2", CpuInfo::SSE4_2 },
-  { "popcnt", CpuInfo::POPCNT },
-  { "avx",    CpuInfo::AVX },
-  { "avx2",   CpuInfo::AVX2 },
+  { "ssse3",     CpuInfo::SSSE3 },
+  { "sse4_1",    CpuInfo::SSE4_1 },
+  { "sse4_2",    CpuInfo::SSE4_2 },
+  { "popcnt",    CpuInfo::POPCNT },
+  { "avx",       CpuInfo::AVX },
+  { "avx2",      CpuInfo::AVX2 },
+  { "pclmuldqd", CpuInfo::PCLMULQDQ }
 };
 static const long num_flags = sizeof(flag_mappings) / sizeof(flag_mappings[0]);
 

http://git-wip-us.apache.org/repos/asf/impala/blob/5aa28b1b/be/src/util/cpu-info.h
----------------------------------------------------------------------
diff --git a/be/src/util/cpu-info.h b/be/src/util/cpu-info.h
index 38d6782..e60babc 100644
--- a/be/src/util/cpu-info.h
+++ b/be/src/util/cpu-info.h
@@ -34,12 +34,13 @@ namespace impala {
 /// /sys/devices)
 class CpuInfo {
  public:
-  static const int64_t SSSE3   = (1 << 1);
-  static const int64_t SSE4_1  = (1 << 2);
-  static const int64_t SSE4_2  = (1 << 3);
-  static const int64_t POPCNT  = (1 << 4);
-  static const int64_t AVX     = (1 << 5);
-  static const int64_t AVX2    = (1 << 6);
+  static const int64_t SSSE3     = (1 << 1);
+  static const int64_t SSE4_1    = (1 << 2);
+  static const int64_t SSE4_2    = (1 << 3);
+  static const int64_t POPCNT    = (1 << 4);
+  static const int64_t AVX       = (1 << 5);
+  static const int64_t AVX2      = (1 << 6);
+  static const int64_t PCLMULQDQ = (1 << 7);
 
   /// Cache enums for L1 (data), L2 and L3
   enum CacheLevel {

http://git-wip-us.apache.org/repos/asf/impala/blob/5aa28b1b/be/src/util/openssl-util-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/openssl-util-test.cc b/be/src/util/openssl-util-test.cc
index 8d98b0d..76f65a5 100644
--- a/be/src/util/openssl-util-test.cc
+++ b/be/src/util/openssl-util-test.cc
@@ -44,6 +44,41 @@ class OpenSSLUtilTest : public ::testing::Test {
     }
   }
 
+  /// Fill arbitrary-length buffer with random bytes
+  void GenerateRandomBytes(uint8_t* data, int64_t len) {
+    DCHECK_GE(len, 0);
+    for (int64_t i = 0; i < len; i++) {
+      data[i] = uniform_int_distribution<uint8_t>(0, UINT8_MAX)(rng_);
+    }
+  }
+
+  void TestEncryptionDecryption(const int64_t buffer_size) {
+    vector<uint8_t> original(buffer_size);
+    vector<uint8_t> scratch(buffer_size); // Scratch buffer for in-place encryption.
+    if (buffer_size % 8 == 0) {
+      GenerateRandomData(original.data(), buffer_size);
+    } else {
+      GenerateRandomBytes(original.data(), buffer_size);
+    }
+
+    // Check all the modes
+    AES_CIPHER_MODE modes[] = {AES_256_GCM, AES_256_CTR, AES_256_CFB};
+    for (auto m : modes) {
+      memcpy(scratch.data(), original.data(), buffer_size);
+
+      EncryptionKey key;
+      key.InitializeRandom();
+      key.SetCipherMode(m);
+
+      ASSERT_OK(key.Encrypt(scratch.data(), buffer_size, scratch.data()));
+      // Check that encryption did something
+      ASSERT_NE(0, memcmp(original.data(), scratch.data(), buffer_size));
+      ASSERT_OK(key.Decrypt(scratch.data(), buffer_size, scratch.data()));
+      // Check that we get the original data back.
+      ASSERT_EQ(0, memcmp(original.data(), scratch.data(), buffer_size));
+    }
+  }
+
   mt19937_64 rng_;
 };
 
@@ -57,7 +92,7 @@ TEST_F(OpenSSLUtilTest, Encryption) {
   GenerateRandomData(original.data(), buffer_size);
 
   // Check both CTR & CFB
-  AES_CIPHER_MODE modes[] = {AES_256_CTR, AES_256_CFB};
+  AES_CIPHER_MODE modes[] = {AES_256_GCM, AES_256_CTR, AES_256_CFB};
   for (auto m : modes) {
     // Iterate multiple times to ensure that key regeneration works correctly.
     EncryptionKey key;
@@ -85,44 +120,42 @@ TEST_F(OpenSSLUtilTest, Encryption) {
 /// Test that encryption and decryption work in-place.
 TEST_F(OpenSSLUtilTest, EncryptInPlace) {
   const int buffer_size = 1024 * 1024;
-  vector<uint8_t> original(buffer_size);
-  vector<uint8_t> scratch(buffer_size); // Scratch buffer for in-place encryption.
-
-  EncryptionKey key;
-  // Check both CTR & CFB
-  AES_CIPHER_MODE modes[] = {AES_256_CTR, AES_256_CFB};
-  for (auto m : modes) {
-    GenerateRandomData(original.data(), buffer_size);
-    memcpy(scratch.data(), original.data(), buffer_size);
-
-    key.InitializeRandom();
-    key.SetCipherMode(m);
-
-    ASSERT_OK(key.Encrypt(scratch.data(), buffer_size, scratch.data()));
-    // Check that encryption did something
-    ASSERT_NE(0, memcmp(original.data(), scratch.data(), buffer_size));
-    ASSERT_OK(key.Decrypt(scratch.data(), buffer_size, scratch.data()));
-    // Check that we get the original data back.
-    ASSERT_EQ(0, memcmp(original.data(), scratch.data(), buffer_size));
-  }
+  TestEncryptionDecryption(buffer_size);
 }
 
 /// Test that encryption works with buffer lengths that don't fit in a 32-bit integer.
 TEST_F(OpenSSLUtilTest, EncryptInPlaceHugeBuffer) {
   const int64_t buffer_size = 3 * 1024L * 1024L * 1024L;
-  vector<uint8_t> original(buffer_size);
-  vector<uint8_t> scratch(buffer_size); // Scratch buffer for in-place encryption.
-  GenerateRandomData(original.data(), buffer_size);
-  memcpy(scratch.data(), original.data(), buffer_size);
+  TestEncryptionDecryption(buffer_size);
+}
+
+/// Test that encryption works with arbitrary-length buffer
+TEST_F(OpenSSLUtilTest, EncryptArbitraryLength) {
+  std::uniform_int_distribution<uint64_t> dis(0, 1024 * 1024);
+  const int buffer_size = dis(rng_);
+  TestEncryptionDecryption(buffer_size);
+}
+
+/// Test integrity in GCM mode
+TEST_F(OpenSSLUtilTest, GcmIntegrity) {
+  const int buffer_size = 1024 * 1024;
+  vector<uint8_t> buffer(buffer_size);
 
   EncryptionKey key;
   key.InitializeRandom();
-  ASSERT_OK(key.Encrypt(scratch.data(), buffer_size, scratch.data()));
-  // Check that encryption did something
-  ASSERT_NE(0, memcmp(original.data(), scratch.data(), buffer_size));
-  ASSERT_OK(key.Decrypt(scratch.data(), buffer_size, scratch.data()));
-  // Check that we get the original data back.
-  ASSERT_EQ(0, memcmp(original.data(), scratch.data(), buffer_size));
+  key.SetCipherMode(AES_256_GCM);
+
+  // Even it has been set as GCM mode, it may fall back to other modes.
+  // Check if GCM mode is supported at runtime.
+  if (key.IsGcmMode()) {
+    GenerateRandomData(buffer.data(), buffer_size);
+    ASSERT_OK(key.Encrypt(buffer.data(), buffer_size, buffer.data()));
+
+    // tamper the data
+    ++buffer[0];
+    Status s = key.Decrypt(buffer.data(), buffer_size, buffer.data());
+    EXPECT_STR_CONTAINS(s.GetDetail(), "EVP_DecryptFinal");
+  }
 }
 
 /// Test basic integrity hash functionality.

http://git-wip-us.apache.org/repos/asf/impala/blob/5aa28b1b/be/src/util/openssl-util.cc
----------------------------------------------------------------------
diff --git a/be/src/util/openssl-util.cc b/be/src/util/openssl-util.cc
index 69dc676..83cd8fd 100644
--- a/be/src/util/openssl-util.cc
+++ b/be/src/util/openssl-util.cc
@@ -20,6 +20,7 @@
 #include <limits.h>
 #include <sstream>
 
+#include <glog/logging.h>
 #include <openssl/err.h>
 #include <openssl/evp.h>
 #include <openssl/rand.h>
@@ -30,12 +31,32 @@
 #include "gutil/strings/substitute.h"
 
 #include "common/names.h"
+#include "cpu-info.h"
 
 DECLARE_string(ssl_client_ca_certificate);
 DECLARE_string(ssl_server_certificate);
 DECLARE_string(ssl_private_key);
 DECLARE_string(ssl_cipher_list);
 
+/// OpenSSL 1.0.1d
+#define OPENSSL_VERSION_1_0_1D 0x1000104fL
+
+/// If not defined at compile time, define them manually
+/// see: openssl/evp.h
+#ifndef EVP_CIPH_GCM_MODE
+#define EVP_CTRL_GCM_SET_IVLEN 0x9
+#define EVP_CTRL_GCM_GET_TAG 0x10
+#define EVP_CTRL_GCM_SET_TAG 0x11
+#endif
+
+extern "C" {
+ATTRIBUTE_WEAK
+const EVP_CIPHER* EVP_aes_256_ctr();
+
+ATTRIBUTE_WEAK
+const EVP_CIPHER* EVP_aes_256_gcm();
+}
+
 namespace impala {
 
 // Counter to track the number of encryption keys generated. Incremented before each key
@@ -107,19 +128,20 @@ void EncryptionKey::InitializeRandom() {
   }
   RAND_bytes(key_, sizeof(key_));
   RAND_bytes(iv_, sizeof(iv_));
+  memset(gcm_tag_, 0, sizeof(gcm_tag_));
   initialized_ = true;
 }
 
-Status EncryptionKey::Encrypt(const uint8_t* data, int64_t len, uint8_t* out) const {
+Status EncryptionKey::Encrypt(const uint8_t* data, int64_t len, uint8_t* out) {
   return EncryptInternal(true, data, len, out);
 }
 
-Status EncryptionKey::Decrypt(const uint8_t* data, int64_t len, uint8_t* out) const {
+Status EncryptionKey::Decrypt(const uint8_t* data, int64_t len, uint8_t* out) {
   return EncryptInternal(false, data, len, out);
 }
 
 Status EncryptionKey::EncryptInternal(
-    bool encrypt, const uint8_t* data, int64_t len, uint8_t* out) const {
+    bool encrypt, const uint8_t* data, int64_t len, uint8_t* out) {
   DCHECK(initialized_);
   DCHECK_GE(len, 0);
   // Create and initialize the context for encryption
@@ -127,6 +149,10 @@ Status EncryptionKey::EncryptInternal(
   EVP_CIPHER_CTX_init(&ctx);
   EVP_CIPHER_CTX_set_padding(&ctx, 0);
 
+  if (IsGcmMode()) {
+    EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_SET_IVLEN, AES_BLOCK_SIZE, NULL);
+  }
+
   // Start encryption/decryption.  We use a 256-bit AES key, and the cipher block mode
   // is either CTR or CFB(stream cipher), both of which support arbitrary length
   // ciphertexts - it doesn't have to be a multiple of 16 bytes. Additionally, CTR
@@ -157,6 +183,11 @@ Status EncryptionKey::EncryptInternal(
     offset += in_len;
   }
 
+  if (IsGcmMode() && !encrypt) {
+    // Set expected tag value
+    EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_SET_TAG, AES_BLOCK_SIZE, gcm_tag_);
+  }
+
   // Finalize encryption or decryption.
   int final_out_len;
   success = encrypt ? EVP_EncryptFinal_ex(&ctx, out + offset, &final_out_len) :
@@ -164,21 +195,74 @@ Status EncryptionKey::EncryptInternal(
   if (success != 1) {
     return OpenSSLErr(encrypt ? "EVP_EncryptFinal" : "EVP_DecryptFinal");
   }
-  // Again safe due to CTR/CFB with no padding
+
+  if (IsGcmMode() && encrypt) {
+    EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_GET_TAG, AES_BLOCK_SIZE, gcm_tag_);
+  }
+
+  // Again safe due to GCM/CTR/CFB with no padding
   DCHECK_EQ(final_out_len, 0);
   return Status::OK();
 }
 
-extern "C" {
-ATTRIBUTE_WEAK
-const EVP_CIPHER* EVP_aes_256_ctr();
-}
-
 const EVP_CIPHER* EncryptionKey::GetCipher() const {
   // use weak symbol to avoid compiling error on OpenSSL 1.0.0 environment
-  if (mode_ == AES_256_CTR && EVP_aes_256_ctr) return EVP_aes_256_ctr();
+  if (mode_ == AES_256_CTR) return EVP_aes_256_ctr();
+  if (mode_ == AES_256_GCM) return EVP_aes_256_gcm();
 
-  // otherwise, fallback to CFB mode
   return EVP_aes_256_cfb();
 }
+
+void EncryptionKey::SetCipherMode(AES_CIPHER_MODE m) {
+  mode_ = m;
+
+  if (!IsModeSupported(m)) {
+    mode_ = GetSupportedDefaultMode();
+    LOG(WARNING) << Substitute("$0 is not supported, fall back to $1.",
+        ModeToString(m), ModeToString(mode_));
+  }
+}
+
+bool EncryptionKey::IsModeSupported(AES_CIPHER_MODE m) const {
+  switch (m) {
+    case AES_256_GCM:
+      // It becomes a bit tricky for GCM mode, because GCM mode is enabled since
+      // OpenSSL 1.0.1, but the tag validation only works since 1.0.1d. We have
+      // to make sure that OpenSSL version >= 1.0.1d for GCM. So we need
+      // SSLeay(). Note that SSLeay() may return the compiling version on
+      // certain platforms if it was built against an older version(see:
+      // IMPALA-6418). In this case, it will return false, and EncryptionKey
+      // will try to fall back to CTR mode, so it is not ideal but is OK to use
+      // SSLeay() for GCM mode here since in the worst case, we will be using
+      // AES_256_CTR in a system that supports AES_256_GCM.
+      return (CpuInfo::IsSupported(CpuInfo::PCLMULQDQ)
+          && SSLeay() >= OPENSSL_VERSION_1_0_1D && EVP_aes_256_gcm);
+
+    case AES_256_CTR:
+      // If TLS1.2 is supported, then we're on a verison of OpenSSL that
+      // supports AES-256-CTR.
+      return (MaxSupportedTlsVersion() >= TLS1_2_VERSION && EVP_aes_256_ctr);
+
+    case AES_256_CFB:
+      return true;
+
+    default:
+      return false;
+  }
+}
+
+AES_CIPHER_MODE EncryptionKey::GetSupportedDefaultMode() const {
+  if (IsModeSupported(AES_256_GCM)) return AES_256_GCM;
+  if (IsModeSupported(AES_256_CTR)) return AES_256_CTR;
+  return AES_256_CFB;
+}
+
+const string EncryptionKey::ModeToString(AES_CIPHER_MODE m) const {
+  switch(m) {
+    case AES_256_GCM: return "AES-GCM";
+    case AES_256_CTR: return "AES-CTR";
+    case AES_256_CFB: return "AES-CFB";
+  }
+  return "Unknown mode";
+}
 }

http://git-wip-us.apache.org/repos/asf/impala/blob/5aa28b1b/be/src/util/openssl-util.h
----------------------------------------------------------------------
diff --git a/be/src/util/openssl-util.h b/be/src/util/openssl-util.h
index 7b1b28e..ef53425 100644
--- a/be/src/util/openssl-util.h
+++ b/be/src/util/openssl-util.h
@@ -60,9 +60,9 @@ bool IsExternalTlsConfigured();
 void SeedOpenSSLRNG();
 
 enum AES_CIPHER_MODE {
-  AES_256_CTR,
   AES_256_CFB,
-  AES_256_GCM // not supported now.
+  AES_256_CTR,
+  AES_256_GCM
 };
 
 /// The hash of a data buffer used for checking integrity. A SHA256 hash is used
@@ -83,43 +83,56 @@ class IntegrityHash {
 /// The key and initialization vector (IV) required to encrypt and decrypt a buffer of
 /// data. This should be regenerated for each buffer of data.
 ///
-/// We use AES with a 256-bit key and CTR/CFB cipher block mode, which gives us a stream
-/// cipher that can support arbitrary-length ciphertexts. If OpenSSL version at runtime
-/// is 1.0.1 or above, CTR mode is used, otherwise CFB mode is used. The IV is used as
+/// We use AES with a 256-bit key and GCM/CTR/CFB cipher block mode, which gives us a
+/// stream cipher that can support arbitrary-length ciphertexts. The mode is chosen
+/// depends on the OpenSSL version & the hardware support at runtime. The IV is used as
 /// an input to the cipher as the "block to supply before the first block of plaintext".
 /// This is required because all ciphers (except the weak ECB) are built such that each
 /// block depends on the output from the previous block. Since the first block doesn't
 /// have a previous block, we supply this IV. Think of it  as starting off the chain of
 /// encryption.
+///
+/// Notes for GCM:
+/// (1) GCM mode was supported since OpenSSL 1.0.1, however the tag verification
+/// in decryption was only supported since OpenSSL 1.0.1d.
+/// (2) The plaintext and the Additional Authenticated Data(AAD) are the two
+/// categories of data that GCM protects. GCM protects the authenticity of the
+/// plaintext and the AAD, and GCM also protects the confidentiality of the
+/// plaintext. The AAD itself is not required or won't change the security.
+/// In our case(Spill to Disk), we just ignore the AAD.
+
 class EncryptionKey {
  public:
-  EncryptionKey() : initialized_(false) {
-    // If TLS1.2 is supported, then we're on a verison of OpenSSL that supports
-    // AES-256-CTR.
-    mode_ = MaxSupportedTlsVersion() < TLS1_2_VERSION ? AES_256_CFB : AES_256_CTR;
-  }
-
-  /// Initialize a key for temporary use with randomly generated data. Reinitializes with
-  /// new random values if the key was already initialized. We use AES-CTR/AES-CFB mode
-  /// so key/IV pairs should not be reused. This function automatically reseeds the RNG
-  /// periodically, so callers do not need to do it.
+  EncryptionKey() : initialized_(false) { mode_ = GetSupportedDefaultMode(); }
+
+  /// Initializes a key for temporary use with randomly generated data, and clears the
+  /// tag for GCM mode. Reinitializes with new random values if the key was already
+  /// initialized. We use AES-GCM/AES-CTR/AES-CFB mode so key/IV pairs should not be
+  /// reused. This function automatically reseeds the RNG periodically, so callers do
+  /// not need to do it.
   void InitializeRandom();
 
   /// Encrypts a buffer of input data 'data' of length 'len' into an output buffer 'out'.
   /// Exactly 'len' bytes will be written to 'out'. This key must be initialized before
   /// calling. Operates in-place if 'in' == 'out', otherwise the buffers must not overlap.
-  Status Encrypt(const uint8_t* data, int64_t len, uint8_t* out) const WARN_UNUSED_RESULT;
+  /// For GCM mode, the hash tag will be kept inside(gcm_tag_ variable).
+  Status Encrypt(const uint8_t* data, int64_t len, uint8_t* out) WARN_UNUSED_RESULT;
 
   /// Decrypts a buffer of input data 'data' of length 'len' that was encrypted with this
   /// key into an output buffer 'out'. Exactly 'len' bytes will be written to 'out'.
   /// This key must be initialized before calling. Operates in-place if 'in' == 'out',
-  /// otherwise the buffers must not overlap.
-  Status Decrypt(const uint8_t* data, int64_t len, uint8_t* out) const WARN_UNUSED_RESULT;
+  /// otherwise the buffers must not overlap. For GCM mode, the hash tag, which is
+  /// computed during encryption, will be used for intgerity verification.
+  Status Decrypt(const uint8_t* data, int64_t len, uint8_t* out) WARN_UNUSED_RESULT;
 
   /// Specify a cipher mode. Currently used only for testing but maybe in future we
   /// can provide a configuration option for the end user who can choose a preferred
   /// mode(GCM, CTR, CFB...) based on their software/hardware environment.
-  void SetCipherMode(AES_CIPHER_MODE m) { mode_ = m; }
+  /// If not supported, fall back to the supported mode at runtime.
+  void SetCipherMode(AES_CIPHER_MODE m);
+
+  /// If is GCM mode at runtime
+  bool IsGcmMode() const { return mode_ == AES_256_GCM; }
 
  private:
   /// Helper method that encrypts/decrypts if 'encrypt' is true/false respectively.
@@ -128,13 +141,25 @@ class EncryptionKey {
   /// This key must be initialized before calling. Operates in-place if 'in' == 'out',
   /// otherwise the buffers must not overlap.
   Status EncryptInternal(bool encrypt, const uint8_t* data, int64_t len,
-      uint8_t* out) const WARN_UNUSED_RESULT;
+      uint8_t* out) WARN_UNUSED_RESULT;
+
+  /// Check if mode m is supported at runtime
+  bool IsModeSupported(AES_CIPHER_MODE m) const;
+
+  /// Returns the a default mode which is supported at runtime. If GCM mode
+  /// is supported, return AES_256_GCM as the default. If GCM is not supported,
+  /// but CTR is still supported, return AES_256_CTR. When both GCM and
+  /// CTR modes are not supported, return AES_256_CFB.
+  AES_CIPHER_MODE GetSupportedDefaultMode() const;
+
+  /// Converts mode type to string.
+  const string ModeToString(AES_CIPHER_MODE m) const;
 
   /// Track whether this key has been initialized, to avoid accidentally using
   /// uninitialized keys.
   bool initialized_;
 
-  /// return a EVP_CIPHER according to cipher mode at runtime
+  /// Returns a EVP_CIPHER according to cipher mode at runtime
   const EVP_CIPHER* GetCipher() const;
 
   /// An AES 256-bit key.
@@ -143,6 +168,9 @@ class EncryptionKey {
   /// An initialization vector to feed as the first block to AES.
   uint8_t iv_[AES_BLOCK_SIZE];
 
+  /// Tag for GCM mode
+  uint8_t gcm_tag_[AES_BLOCK_SIZE];
+
   /// Cipher Mode
   AES_CIPHER_MODE mode_;
 };


[4/6] impala git commit: IMPALA-6477: Disable rpc-mgr-kerberized-test

Posted by ta...@apache.org.
IMPALA-6477: Disable rpc-mgr-kerberized-test

This test is breaking builds on CentOS 6.4. Disable it until we
find out the root cause.

Change-Id: I5654d8c41b130544a8f9bc38f69433fcc171eca0
Reviewed-on: http://gerrit.cloudera.org:8080/9256
Reviewed-by: Alex Behm <al...@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/9233a8e9
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/9233a8e9
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/9233a8e9

Branch: refs/heads/master
Commit: 9233a8e9751bceacc176f1644737526141694edd
Parents: 07965a4
Author: Sailesh Mukil <sa...@cloudera.com>
Authored: Thu Feb 8 09:15:48 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Feb 9 02:15:58 2018 +0000

----------------------------------------------------------------------
 be/src/rpc/rpc-mgr-kerberized-test.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/9233a8e9/be/src/rpc/rpc-mgr-kerberized-test.cc
----------------------------------------------------------------------
diff --git a/be/src/rpc/rpc-mgr-kerberized-test.cc b/be/src/rpc/rpc-mgr-kerberized-test.cc
index 57ed3eb..08c1971 100644
--- a/be/src/rpc/rpc-mgr-kerberized-test.cc
+++ b/be/src/rpc/rpc-mgr-kerberized-test.cc
@@ -51,10 +51,11 @@ class RpcMgrKerberizedTest :
   boost::scoped_ptr<MiniKdcWrapper> kdc_wrapper_;
 };
 
-INSTANTIATE_TEST_CASE_P(KerberosOnAndOff,
+// TODO: IMPALA-6477: This test breaks on CentOS 6.4. Re-enable after a fix.
+/*INSTANTIATE_TEST_CASE_P(KerberosOnAndOff,
                         RpcMgrKerberizedTest,
                         ::testing::Values(USE_KUDU_KERBEROS,
-                                          USE_IMPALA_KERBEROS));
+                                          USE_IMPALA_KERBEROS));*/
 
 TEST_P(RpcMgrKerberizedTest, MultipleServicesTls) {
   // TODO: We're starting a seperate RpcMgr here instead of configuring


[6/6] impala git commit: IMPALA-6495: fix targeted-perf for new column alias syntax

Posted by ta...@apache.org.
IMPALA-6495: fix targeted-perf for new column alias syntax

I was able to run bin/single_node_perf_run.py after these changes.

Change-Id: Ib25139873b1c67f8039ac6c85e936135e008101b
Reviewed-on: http://gerrit.cloudera.org:8080/9268
Reviewed-by: Philip Zeyliger <ph...@cloudera.com>
Tested-by: Tim Armstrong <ta...@cloudera.com>


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

Branch: refs/heads/master
Commit: 3151421902e1373d483dd784f4241bf3caa0ceea
Parents: 5aa28b1
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Wed Feb 7 08:46:45 2018 -0800
Committer: Tim Armstrong <ta...@cloudera.com>
Committed: Fri Feb 9 16:49:54 2018 +0000

----------------------------------------------------------------------
 .../primitive_shuffle_join_one_to_many_string_with_groupby.test    | 2 +-
 .../queries/primitive_shuffle_join_union_all_with_groupby.test     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/31514219/testdata/workloads/targeted-perf/queries/primitive_shuffle_join_one_to_many_string_with_groupby.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/targeted-perf/queries/primitive_shuffle_join_one_to_many_string_with_groupby.test b/testdata/workloads/targeted-perf/queries/primitive_shuffle_join_one_to_many_string_with_groupby.test
index fd90d8a..3ca78cf 100644
--- a/testdata/workloads/targeted-perf/queries/primitive_shuffle_join_one_to_many_string_with_groupby.test
+++ b/testdata/workloads/targeted-perf/queries/primitive_shuffle_join_one_to_many_string_with_groupby.test
@@ -11,7 +11,7 @@ JOIN /* +shuffle */
   ( SELECT upper(concat(cast(o_orderkey AS string),'bla')) o_orderkey_string
    FROM orders) o ON l.l_orderkey_string = o.o_orderkey_string
 GROUP BY o.o_orderkey_string
-HAVING cnt = 999999;
+HAVING count(*) = 999999;
 ---- RESULTS
 ---- TYPES
 ====

http://git-wip-us.apache.org/repos/asf/impala/blob/31514219/testdata/workloads/targeted-perf/queries/primitive_shuffle_join_union_all_with_groupby.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/targeted-perf/queries/primitive_shuffle_join_union_all_with_groupby.test b/testdata/workloads/targeted-perf/queries/primitive_shuffle_join_union_all_with_groupby.test
index 6295274..4aebb80 100644
--- a/testdata/workloads/targeted-perf/queries/primitive_shuffle_join_union_all_with_groupby.test
+++ b/testdata/workloads/targeted-perf/queries/primitive_shuffle_join_union_all_with_groupby.test
@@ -20,7 +20,7 @@ FROM (
     GROUP BY l_orderkey
     ) a
 GROUP BY l_orderkey
-HAVING ROWCOUNT = 99999999;
+HAVING count(*) = 99999999;
 ---- RESULTS
 ---- TYPES
 ====


[3/6] impala git commit: IMPALA-6486: Fix INVALIDATE METADATA hang after statestore restart

Posted by ta...@apache.org.
IMPALA-6486: Fix INVALIDATE METADATA hang after statestore restart

This commit fixes an issue where an INVALIDATE METADATA statement will
hang if executed after a statestore restart. The issue was that the
CatalogObjectVersionQueue was not properly reset when a non-delta
catalog topic update was received in an impalad, causing it to record
duplicate catalog versions and, hence, not properly advancing the minimum
catalog object watermark. The latter is used by INVALIDATE METADATA to
determine when the effects of that operation have been applied to the
local impalad catalog cache.

Change-Id: Icf3ee31c28884ed79c5dbc700ccd4ef761015c68
Reviewed-on: http://gerrit.cloudera.org:8080/9232
Reviewed-by: Dimitris Tsirogiannis <dt...@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/07965a4c
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/07965a4c
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/07965a4c

Branch: refs/heads/master
Commit: 07965a4c816c016a88e066d7f09a81ab2d86d91f
Parents: 8abc166
Author: Dimitris Tsirogiannis <dt...@cloudera.com>
Authored: Tue Feb 6 15:11:53 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Feb 8 21:42:57 2018 +0000

----------------------------------------------------------------------
 .../org/apache/impala/catalog/CatalogObjectVersionQueue.java  | 2 ++
 .../main/java/org/apache/impala/catalog/ImpaladCatalog.java   | 7 +++----
 2 files changed, 5 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/07965a4c/fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionQueue.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionQueue.java b/fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionQueue.java
index 5fcd398..e24e66b 100644
--- a/fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionQueue.java
+++ b/fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionQueue.java
@@ -70,4 +70,6 @@ public class CatalogObjectVersionQueue {
       removeVersion(catalogObject.getCatalogVersion());
     }
   }
+
+  public void clear() { objectVersions_.clear(); }
 }

http://git-wip-us.apache.org/repos/asf/impala/blob/07965a4c/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java b/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
index 79960e4..99bd23e 100644
--- a/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
@@ -97,13 +97,12 @@ public class ImpaladCatalog extends Catalog {
   // Used during table creation.
   private final String defaultKuduMasterHosts_;
 
-  /**
-   * C'tor used by tests that need to validate the ImpaladCatalog outside of the
-   * CatalogServer.
-   */
   public ImpaladCatalog(String defaultKuduMasterHosts) {
     super();
     defaultKuduMasterHosts_ = defaultKuduMasterHosts;
+    // Ensure the contents of the CatalogObjectVersionQueue instance are cleared when a
+    // new instance of ImpaladCatalog is created (see IMPALA-6486).
+    CatalogObjectVersionQueue.INSTANCE.clear();
   }
 
   /**


[2/6] impala git commit: IMPALA-6477: Fix flakiness with thrift-server-test

Posted by ta...@apache.org.
IMPALA-6477: Fix flakiness with thrift-server-test

THe ThriftKerberizedParamsTest.SslConnectivity backend test verifies
that when kerberos and TLS are switched on, we cannot perform a
negotiation with a non TLS enabled client.

The test works as expected, however, the error messages can be
different in some scenarios. This patch just updates the test to
expect a different error message that has the same effect.

This change also links KRB5_REALM_OVERRIDE back with the
rpc-mgr-test, as one test run showed a similar failure as
IMPALA-6268. It's possible that this is due to IMPALA-6448.

Change-Id: I6294447416ecccc864b842013487f4d93afadc6b
Reviewed-on: http://gerrit.cloudera.org:8080/9247
Reviewed-by: Alex Behm <al...@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/8abc166e
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/8abc166e
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/8abc166e

Branch: refs/heads/master
Commit: 8abc166e113a54e7921796fd1ddfebf6bfdc3982
Parents: 572af61
Author: Sailesh Mukil <sa...@cloudera.com>
Authored: Wed Feb 7 13:26:05 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Feb 8 20:22:26 2018 +0000

----------------------------------------------------------------------
 be/src/rpc/CMakeLists.txt        |  1 +
 be/src/rpc/thrift-server-test.cc | 10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/8abc166e/be/src/rpc/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/be/src/rpc/CMakeLists.txt b/be/src/rpc/CMakeLists.txt
index e4d96e2..1c7adc6 100644
--- a/be/src/rpc/CMakeLists.txt
+++ b/be/src/rpc/CMakeLists.txt
@@ -50,6 +50,7 @@ ADD_BE_TEST(rpc-mgr-test)
 add_dependencies(rpc-mgr-test rpc_test_proto)
 target_link_libraries(rpc-mgr-test rpc_test_proto)
 target_link_libraries(rpc-mgr-test security-test-for-impala)
+target_link_libraries(rpc-mgr-test ${KRB5_REALM_OVERRIDE})
 
 ADD_BE_TEST(rpc-mgr-kerberized-test)
 add_dependencies(rpc-mgr-kerberized-test rpc_test_proto)

http://git-wip-us.apache.org/repos/asf/impala/blob/8abc166e/be/src/rpc/thrift-server-test.cc
----------------------------------------------------------------------
diff --git a/be/src/rpc/thrift-server-test.cc b/be/src/rpc/thrift-server-test.cc
index f1e0f4b..8bd7275 100644
--- a/be/src/rpc/thrift-server-test.cc
+++ b/be/src/rpc/thrift-server-test.cc
@@ -172,8 +172,14 @@ TEST_P(ThriftKerberizedParamsTest, SslConnectivity) {
     // When Kerberos is ON, the SASL negotiation happens inside Open(). We expect that to
     // fail beacuse the server expects the client to negotiate over an encrypted
     // connection.
-    EXPECT_STR_CONTAINS(non_ssl_client.Open().GetDetail(),
-        "No more data to read");
+    // The expected error message can either state "No more data to read" or
+    // "Couldn't open transport".
+    const std::string& status = non_ssl_client.Open().GetDetail();
+    size_t found_substr = status.find("No more data to read");
+    if (found_substr == string::npos) {
+      EXPECT_STR_CONTAINS(non_ssl_client.Open().GetDetail(),
+          "Couldn't open transport");
+    }
   }
 
 }