IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p4

Dependency changes:
- BE and python use thrift 0.9.3-p4 from native-toolchain.
- FE uses thrift 0.9.3 from apache maven repo.
- Fb303 and http components dependencies are no longer needed in FE and
  are removed.
- The minimum openssl version requirement is increased to 1.0.1.

Configuration change:
- Thrift codegen option movable_type is enabled. New code no longer
  needs to use std::swap to avoid copying.

Cherry-picks: not for 2.x

Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
Reviewed-on: http://gerrit.cloudera.org:8080/9300
Reviewed-by: Tianyi Wang <twang@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>

CMakeLists.txt

@@ -156,7 +156,7 @@ message(STATUS "Boost libraries: " ${Boost_LIBRARIES})
 # platform that this build might ultimately be deployed on. If no suitable OpenSSL is
 # found, use the toolchain version - but beware that Impala will then fail to start if
 # deployed on a system with a lower version.
-find_package(OpenSSL 1.0.0 QUIET)
+find_package(OpenSSL 1.0.1 QUIET)
 if (NOT ${OPENSSL_FOUND})
   set_dep_root(OPENSSL)
   set(OPENSSL_FOUND ON)

be/src/common/init.cc

@@ -17,6 +17,7 @@
 
 #include "common/init.h"
 
+#include <csignal>
 #include <gperftools/heap-profiler.h>
 #include <gperftools/malloc_extension.h>
 
@@ -232,6 +233,11 @@ void impala::InitCommonRuntime(int argc, char** argv, bool init_jvm,
   LOG(INFO) << "Using hostname: " << FLAGS_hostname;
   impala::LogCommandLineFlags();
 
+  // When a process calls send(2) on a socket closed on the other end, linux generates
+  // SIGPIPE. MSG_NOSIGNAL can be passed to send(2) to disable it, which thrift does. But
+  // OpenSSL doesn't have place for this parameter so the signal must be disabled
+  // manually.
+  signal(SIGPIPE, SIG_IGN);
   InitThriftLogging();
 
   LOG(INFO) << CpuInfo::DebugString();

be/src/rpc/TAcceptQueueServer.cpp

@@ -22,16 +22,7 @@
 #include "rpc/TAcceptQueueServer.h"
 
 #include <thrift/concurrency/PlatformThreadFactory.h>
-#include <thrift/transport/TTransportException.h>
 
-#include <iostream>
-#include <string>
-
-#ifdef HAVE_UNISTD_H
-#include <unistd.h>
-#endif
-
-#include "common/status.h"
 #include "util/thread-pool.h"
 
 DEFINE_int32(accepted_cnxn_queue_depth, 10000,
@@ -56,22 +47,22 @@ class TAcceptQueueServer::Task : public Runnable {
       shared_ptr<TProtocol> input, shared_ptr<TProtocol> output,
       shared_ptr<TTransport> transport)
     : server_(server),
-      processor_(processor),
-      input_(input),
-      output_(output),
-      transport_(transport) {}
+      processor_(std::move(processor)),
+      input_(std::move(input)),
+      output_(std::move(output)),
+      transport_(std::move(transport)) {}
 
-  ~Task() {}
+  ~Task() override = default;
 
-  void run() {
+  void run() override {
     boost::shared_ptr<TServerEventHandler> eventHandler = server_.getEventHandler();
-    void* connectionContext = NULL;
-    if (eventHandler != NULL) {
+    void* connectionContext = nullptr;
+    if (eventHandler != nullptr) {
       connectionContext = eventHandler->createContext(input_, output_);
     }
     try {
       for (;;) {
-        if (eventHandler != NULL) {
+        if (eventHandler != nullptr) {
           eventHandler->processContext(connectionContext, transport_);
         }
         if (!processor_->process(input_, output_, connectionContext)
@@ -90,7 +81,7 @@ class TAcceptQueueServer::Task : public Runnable {
     } catch (...) {
       GlobalOutput("TAcceptQueueServer uncaught exception.");
     }
-    if (eventHandler != NULL) {
+    if (eventHandler != nullptr) {
       eventHandler->deleteContext(connectionContext, input_, output_);
     }
 
@@ -125,18 +116,27 @@ class TAcceptQueueServer::Task : public Runnable {
   shared_ptr<TTransport> transport_;
 };
 
+TAcceptQueueServer::TAcceptQueueServer(const boost::shared_ptr<TProcessor>& processor,
+    const boost::shared_ptr<TServerTransport>& serverTransport,
+    const boost::shared_ptr<TTransportFactory>& transportFactory,
+    const boost::shared_ptr<TProtocolFactory>& protocolFactory,
+    const boost::shared_ptr<ThreadFactory>& threadFactory,
+    int32_t maxTasks)
+    : TServer(processor, serverTransport, transportFactory, protocolFactory),
+      threadFactory_(threadFactory), maxTasks_(maxTasks) {
+  init();
+}
+
 void TAcceptQueueServer::init() {
   stop_ = false;
   metrics_enabled_ = false;
-  queue_size_metric_ = NULL;
+  queue_size_metric_ = nullptr;
 
   if (!threadFactory_) {
     threadFactory_.reset(new PlatformThreadFactory);
   }
 }
 
-TAcceptQueueServer::~TAcceptQueueServer() {}
-
 // New.
 void TAcceptQueueServer::SetupConnection(boost::shared_ptr<TTransport> client) {
   if (metrics_enabled_) queue_size_metric_->Increment(-1);
@@ -174,25 +174,25 @@ void TAcceptQueueServer::SetupConnection(boost::shared_ptr<TTransport> client) {
     // Start the thread!
     thread->start();
   } catch (TException& tx) {
-    if (inputTransport != NULL) {
+    if (inputTransport != nullptr) {
       inputTransport->close();
     }
-    if (outputTransport != NULL) {
+    if (outputTransport != nullptr) {
       outputTransport->close();
     }
-    if (client != NULL) {
+    if (client != nullptr) {
       client->close();
     }
     string errStr = string("TAcceptQueueServer: Caught TException: ") + tx.what();
     GlobalOutput(errStr.c_str());
   } catch (string s) {
-    if (inputTransport != NULL) {
+    if (inputTransport != nullptr) {
       inputTransport->close();
     }
-    if (outputTransport != NULL) {
+    if (outputTransport != nullptr) {
       outputTransport->close();
     }
-    if (client != NULL) {
+    if (client != nullptr) {
       client->close();
     }
     string errStr = "TAcceptQueueServer: Unknown exception: " + s;
@@ -205,7 +205,7 @@ void TAcceptQueueServer::serve() {
   serverTransport_->listen();
 
   // Run the preServe event
-  if (eventHandler_ != NULL) {
+  if (eventHandler_ != nullptr) {
     eventHandler_->preServe();
   }
 
@@ -283,7 +283,7 @@ void TAcceptQueueServer::serve() {
 }
 
 void TAcceptQueueServer::InitMetrics(MetricGroup* metrics, const string& key_prefix) {
-  DCHECK(metrics != NULL);
+  DCHECK(metrics != nullptr);
   stringstream queue_size_ss;
   queue_size_ss << key_prefix << ".connection-setup-queue-size";
   queue_size_metric_ = metrics->AddGauge(queue_size_ss.str(), 0);

be/src/rpc/TAcceptQueueServer.h

@@ -52,47 +52,18 @@ using apache::thrift::concurrency::ThreadFactory;
 class TAcceptQueueServer : public TServer {
  public:
   class Task;
-
-  // TODO: Determine which c'tors are used and remove unused ones.
-  template <typename ProcessorFactory>
-  TAcceptQueueServer(const boost::shared_ptr<ProcessorFactory>& processorFactory,
-      const boost::shared_ptr<TServerTransport>& serverTransport,
-      const boost::shared_ptr<TTransportFactory>& transportFactory,
-      const boost::shared_ptr<TProtocolFactory>& protocolFactory,
-      int32_t maxTasks = 0,
-      THRIFT_OVERLOAD_IF(ProcessorFactory, TProcessorFactory));
-
-  template <typename ProcessorFactory>
-  TAcceptQueueServer(const boost::shared_ptr<ProcessorFactory>& processorFactory,
+  TAcceptQueueServer(const boost::shared_ptr<TProcessor>& processor,
       const boost::shared_ptr<TServerTransport>& serverTransport,
       const boost::shared_ptr<TTransportFactory>& transportFactory,
       const boost::shared_ptr<TProtocolFactory>& protocolFactory,
       const boost::shared_ptr<ThreadFactory>& threadFactory,
-      int32_t maxTasks = 0,
-      THRIFT_OVERLOAD_IF(ProcessorFactory, TProcessorFactory));
+      int32_t maxTasks = 0);
 
-  template <typename Processor>
-  TAcceptQueueServer(const boost::shared_ptr<Processor>& processor,
-      const boost::shared_ptr<TServerTransport>& serverTransport,
-      const boost::shared_ptr<TTransportFactory>& transportFactory,
-      const boost::shared_ptr<TProtocolFactory>& protocolFactory,
-      int32_t maxTasks = 0,
-      THRIFT_OVERLOAD_IF(Processor, TProcessor));
+  ~TAcceptQueueServer() override = default;
 
-  template <typename Processor>
-  TAcceptQueueServer(const boost::shared_ptr<Processor>& processor,
-      const boost::shared_ptr<TServerTransport>& serverTransport,
-      const boost::shared_ptr<TTransportFactory>& transportFactory,
-      const boost::shared_ptr<TProtocolFactory>& protocolFactory,
-      const boost::shared_ptr<ThreadFactory>& threadFactory,
-      int32_t maxTasks = 0,
-      THRIFT_OVERLOAD_IF(Processor, TProcessor));
+  void serve() override;
 
-  virtual ~TAcceptQueueServer();
-
-  virtual void serve();
-
-  void stop() {
+  void stop() override {
     stop_ = true;
     serverTransport_->interrupt();
   }
@@ -126,57 +97,6 @@ class TAcceptQueueServer : public TServer {
   impala::IntGauge* queue_size_metric_;
 };
 
-template <typename ProcessorFactory>
-TAcceptQueueServer::TAcceptQueueServer(
-    const boost::shared_ptr<ProcessorFactory>& processorFactory,
-    const boost::shared_ptr<TServerTransport>& serverTransport,
-    const boost::shared_ptr<TTransportFactory>& transportFactory,
-    const boost::shared_ptr<TProtocolFactory>& protocolFactory,
-    int32_t maxTasks,
-    THRIFT_OVERLOAD_IF_DEFN(ProcessorFactory, TProcessorFactory))
-  : TServer(processorFactory, serverTransport, transportFactory, protocolFactory),
-    maxTasks_(maxTasks) {
-  init();
-}
-
-template <typename ProcessorFactory>
-TAcceptQueueServer::TAcceptQueueServer(
-    const boost::shared_ptr<ProcessorFactory>& processorFactory,
-    const boost::shared_ptr<TServerTransport>& serverTransport,
-    const boost::shared_ptr<TTransportFactory>& transportFactory,
-    const boost::shared_ptr<TProtocolFactory>& protocolFactory,
-    const boost::shared_ptr<ThreadFactory>& threadFactory,
-    int32_t maxTasks,
-    THRIFT_OVERLOAD_IF_DEFN(ProcessorFactory, TProcessorFactory))
-  : TServer(processorFactory, serverTransport, transportFactory, protocolFactory),
-    threadFactory_(threadFactory), maxTasks_(maxTasks) {
-  init();
-}
-
-template <typename Processor>
-TAcceptQueueServer::TAcceptQueueServer(const boost::shared_ptr<Processor>& processor,
-    const boost::shared_ptr<TServerTransport>& serverTransport,
-    const boost::shared_ptr<TTransportFactory>& transportFactory,
-    const boost::shared_ptr<TProtocolFactory>& protocolFactory,
-    int32_t maxTasks,
-    THRIFT_OVERLOAD_IF_DEFN(Processor, TProcessor))
-  : TServer(processor, serverTransport, transportFactory, protocolFactory),
-    maxTasks_(maxTasks) {
-  init();
-}
-
-template <typename Processor>
-TAcceptQueueServer::TAcceptQueueServer(const boost::shared_ptr<Processor>& processor,
-    const boost::shared_ptr<TServerTransport>& serverTransport,
-    const boost::shared_ptr<TTransportFactory>& transportFactory,
-    const boost::shared_ptr<TProtocolFactory>& protocolFactory,
-    const boost::shared_ptr<ThreadFactory>& threadFactory,
-    int32_t maxTasks,
-    THRIFT_OVERLOAD_IF_DEFN(Processor, TProcessor))
-  : TServer(processor, serverTransport, transportFactory, protocolFactory),
-    threadFactory_(threadFactory), maxTasks_(maxTasks) {
-  init();
-}
 } // namespace server
 } // namespace thrift
 } // namespace apache

be/src/rpc/authentication.cc

@@ -904,7 +904,7 @@ Status NoAuthProvider::WrapClientTransport(const string& hostname,
 }
 
 Status AuthManager::Init() {
-  ssl_socket_factory_.reset(new TSSLSocketFactory());
+  ssl_socket_factory_.reset(new TSSLSocketFactory(TLSv1_0));
 
   bool use_ldap = false;
   const string excl_msg = "--$0 and --$1 are mutually exclusive "

be/src/rpc/thrift-server-test.cc

@@ -331,7 +331,7 @@ TEST(SslTest, MismatchedCiphers) {
 TEST(SslTest, StringToProtocol) {
   SSLProtocol version;
   map<string, SSLProtocol> TEST_CASES = {
-      {"tlsv1", TLSv1_0_plus}, {"tlsv1.1", TLSv1_1_plus}, {"tlsv1.2", TLSv1_2_plus}};
+      {"tlsv1", TLSv1_0}, {"tlsv1.1", TLSv1_1}, {"tlsv1.2", TLSv1_2}};
   for (auto p : TEST_CASES) {
     EXPECT_OK(SSLProtoVersions::StringToProtocol(p.first, &version));
     EXPECT_EQ(p.second, version) << "TLS version: " << p.first;
@@ -351,16 +351,12 @@ TEST(SslTest, TLSVersionControl) {
     set<SSLProtocol> whitelist;
   };
 
-  // Test all configurations supported by Thrift, even if some won't work with the linked
-  // OpenSSL(). We catch those by checking IsSupported() for both the client and ther
-  // server.
-  vector<Config> configs = {{TLSv1_0, {TLSv1_0, TLSv1_0_plus}},
-      {TLSv1_0_plus,
-          {TLSv1_0, TLSv1_1, TLSv1_2, TLSv1_0_plus, TLSv1_1_plus, TLSv1_2_plus}},
-      {TLSv1_1, {TLSv1_1_plus, TLSv1_1, TLSv1_0_plus}},
-      {TLSv1_1_plus, {TLSv1_1, TLSv1_2, TLSv1_0_plus, TLSv1_1_plus, TLSv1_2_plus}},
-      {TLSv1_2, {TLSv1_2, TLSv1_0_plus, TLSv1_1_plus, TLSv1_2_plus}},
-      {TLSv1_2_plus, {TLSv1_2, TLSv1_0_plus, TLSv1_1_plus, TLSv1_2_plus}}};
+  // All configurations supported by linked OpenSSL should work. We catch unsupported
+  // protocols by checking IsSupported() for both the client and the server.
+  vector<Config> configs = {
+      {TLSv1_0, {TLSv1_0, TLSv1_1, TLSv1_2}},
+      {TLSv1_1, {TLSv1_0, TLSv1_1, TLSv1_2}},
+      {TLSv1_2, {TLSv1_0, TLSv1_1, TLSv1_2}}};
 
   for (const auto& config : configs) {
     // For each config, start a server with the requested protocol spec, and then try to

be/src/rpc/thrift-server.cc

@@ -65,10 +65,11 @@ namespace impala {
 
 // Specifies the allowed set of values for --ssl_minimum_version. To keep consistent with
 // Apache Kudu, specifying a single version enables all versions including and succeeding
-// that one (e.g. TLSv1.1 enables v1.1 and v1.2). Specifying TLSv1.1_only enables only
-// v1.1.
+// that one (e.g. TLSv1.1 enables v1.1 and v1.2).
 map<string, SSLProtocol> SSLProtoVersions::PROTO_MAP = {
-    {"tlsv1.2", TLSv1_2_plus}, {"tlsv1.1", TLSv1_1_plus}, {"tlsv1", TLSv1_0_plus}};
+    {"tlsv1.2", TLSv1_2},
+    {"tlsv1.1", TLSv1_1},
+    {"tlsv1", TLSv1_0}};
 
 Status SSLProtoVersions::StringToProtocol(const string& in, SSLProtocol* protocol) {
   for (const auto& proto : SSLProtoVersions::PROTO_MAP) {
@@ -82,15 +83,15 @@ Status SSLProtoVersions::StringToProtocol(const string& in, SSLProtocol* protoco
 }
 
 bool SSLProtoVersions::IsSupported(const SSLProtocol& protocol) {
-  DCHECK_LE(protocol, TLSv1_2_plus);
+  DCHECK_LE(protocol, TLSv1_2);
   int max_supported_tls_version = MaxSupportedTlsVersion();
   DCHECK_GE(max_supported_tls_version, TLS1_VERSION);
 
   switch (max_supported_tls_version) {
     case TLS1_VERSION:
-      return protocol == TLSv1_0_plus || protocol == TLSv1_0;
+      return protocol == TLSv1_0;
     case TLS1_1_VERSION:
-      return protocol != TLSv1_2_plus && protocol != TLSv1_2;
+      return protocol == TLSv1_0 || protocol == TLSv1_1;
     default:
       DCHECK_GE(max_supported_tls_version, TLS1_2_VERSION);
       return true;
@@ -437,7 +438,7 @@ Status ThriftServer::Start() {
   RETURN_IF_ERROR(CreateSocket(&server_socket));
   RETURN_IF_ERROR(auth_provider_->GetServerTransportFactory(&transport_factory));
   server_.reset(new TAcceptQueueServer(processor_, server_socket, transport_factory,
-        protocol_factory, thread_factory, max_concurrent_connections_));
+      protocol_factory, thread_factory, max_concurrent_connections_));
   if (metrics_ != NULL) {
     (static_cast<TAcceptQueueServer*>(server_.get()))->InitMetrics(metrics_,
         Substitute("impala.thrift-server.$0", name_));

be/src/rpc/thrift-server.h

@@ -320,7 +320,7 @@ class ThriftServerBuilder {
 
   bool enable_ssl_ = false;
   apache::thrift::transport::SSLProtocol version_ =
-      apache::thrift::transport::SSLProtocol::TLSv1_0_plus;
+      apache::thrift::transport::SSLProtocol::TLSv1_0;
   std::string certificate_;
   std::string private_key_;
   std::string pem_password_cmd_;

be/src/rpc/thrift-thread.h

@@ -18,13 +18,6 @@
 #ifndef IMPALA_RPC_THRIFT_THREAD_H
 #define IMPALA_RPC_THRIFT_THREAD_H
 
-/// This is required for thrift's Thread.h to compile. Thrift also includes an experimental
-/// Boost-based thread implementation that is enabled by #define USE_BOOST_THREAD. It is
-/// important that USE_BOOST_THREAD is defined if and only if it was defined when compiling
-/// Thrift itself, as it causes the size of id_t to change, which will cause crashes if
-/// it's different sizes in Impala and in Thrift.
-#define HAVE_PTHREAD_H
-
 #include <thrift/concurrency/Thread.h>
 
 #include "common/logging.h"

be/src/rpc/thrift-util.cc

@@ -57,14 +57,14 @@ using namespace apache::thrift::server;
 using namespace apache::thrift::protocol;
 using namespace apache::thrift::concurrency;
 
-// IsRecvTimeoutTException() and IsSendFailTException() make assumption about the
+// IsRecvTimeoutTException() and IsConnResetTException() make assumption about the
 // implementation of read(), write() and write_partial() in TSocket.cpp and those
 // functions may change between different versions of Thrift.
 static_assert(PACKAGE_VERSION[0] == '0', "");
 static_assert(PACKAGE_VERSION[1] == '.', "");
 static_assert(PACKAGE_VERSION[2] == '9', "");
 static_assert(PACKAGE_VERSION[3] == '.', "");
-static_assert(PACKAGE_VERSION[4] == '0', "");
+static_assert(PACKAGE_VERSION[4] == '3', "");
 static_assert(PACKAGE_VERSION[5] == '\0', "");
 
 // Thrift defines operator< but does not implement it. This is a stub

bin/impala-config.sh

@@ -79,7 +79,7 @@ fi
 # moving to a different build of the toolchain, e.g. when a version is bumped or a
 # compile option is changed. The build id can be found in the output of the toolchain
 # build jobs, it is constructed from the build number and toolchain git hash prefix.
-export IMPALA_TOOLCHAIN_BUILD_ID=102-02a8e245df
+export IMPALA_TOOLCHAIN_BUILD_ID=107-764a0ddc79
 # Versions of toolchain dependencies.
 # -----------------------------------
 export IMPALA_AVRO_VERSION=1.7.4-p4
@@ -146,9 +146,9 @@ export IMPALA_TPC_DS_VERSION=2.1.0
 unset IMPALA_TPC_DS_URL
 export IMPALA_TPC_H_VERSION=2.17.0
 unset IMPALA_TPC_H_URL
-export IMPALA_THRIFT_VERSION=0.9.0-p11
+export IMPALA_THRIFT_VERSION=0.9.3-p4
 unset IMPALA_THRIFT_URL
-export IMPALA_THRIFT_JAVA_VERSION=0.9.0
+export IMPALA_THRIFT_JAVA_VERSION=0.9.3
 unset IMPALA_THRIFT_JAVA_URL
 export IMPALA_ZLIB_VERSION=1.2.8
 unset IMPALA_ZLIB_URL

buildall.sh

@@ -347,6 +347,9 @@ bootstrap_dependencies() {
     echo ">>> Downloading and extracting toolchain dependencies."
     "$IMPALA_HOME/bin/bootstrap_toolchain.py"
     echo "Toolchain bootstrap complete."
+
+    # Reset python path to include thrift
+    . "$IMPALA_HOME/bin/set-pythonpath.sh"
   fi
 }
 

common/thrift/CMakeLists.txt

@@ -58,14 +58,18 @@ function(THRIFT_GEN VAR)
     # It depends on hive_meta_store, which in turn depends on fb303.
     # The java dependency is handled by maven.
     # We need to generate C++ src file for the parent dependencies using the "-r" option.
-    set(CPP_ARGS ${THRIFT_INCLUDE_DIR_OPTION} --gen cpp -o ${BE_OUTPUT_DIR})
+    set(CPP_ARGS ${THRIFT_INCLUDE_DIR_OPTION} --gen cpp:moveable_types -o
+        ${BE_OUTPUT_DIR})
     IF (FIL STREQUAL "beeswax.thrift")
-      set(CPP_ARGS -r ${THRIFT_INCLUDE_DIR_OPTION} --gen cpp -o ${BE_OUTPUT_DIR})
+      set(CPP_ARGS -r ${CPP_ARGS})
     ENDIF(FIL STREQUAL "beeswax.thrift")
 
-    IF (FIL STREQUAL ${TCLI_SERVICE_THRIFT} OR FIL STREQUAL "parquet.thrift")
-      # HiveServer2 and Parquet thrift generation
-      # Do not generate Java source because we should use the jar from Hive or Parquet
+    IF (FIL STREQUAL ${TCLI_SERVICE_THRIFT} OR FIL STREQUAL "parquet.thrift" OR
+        FIL STREQUAL "ImpalaService.thrift")
+      # Do not generate Java HiveServer2 and Parquet files because we should use the jar
+      # from Hive or Parquet.
+      # Also do not generate ImpalaService.thrift because the generated code doesn't
+      # compile with hive if the thrift version in hive is 0.9.0
       add_custom_command(
         OUTPUT ${OUTPUT_BE_FILE}
         COMMAND ${THRIFT_COMPILER} ${CPP_ARGS} ${FIL}
@@ -74,7 +78,8 @@ function(THRIFT_GEN VAR)
         COMMENT "Running thrift compiler on ${FIL}"
         VERBATIM
       )
-    ELSE (FIL STREQUAL ${TCLI_SERVICE_THRIFT} OR FIL STREQUAL "parquet.thrift")
+    ELSE (FIL STREQUAL ${TCLI_SERVICE_THRIFT} OR FIL STREQUAL "parquet.thrift" OR
+        FIL STREQUAL "ImpalaService.thrift")
       add_custom_command(
         OUTPUT ${OUTPUT_BE_FILE}
         COMMAND ${THRIFT_COMPILER} ${CPP_ARGS} ${FIL}
@@ -84,7 +89,8 @@ function(THRIFT_GEN VAR)
         COMMENT "Running thrift compiler on ${FIL}"
         VERBATIM
     )
-    ENDIF(FIL STREQUAL ${TCLI_SERVICE_THRIFT} OR FIL STREQUAL "parquet.thrift")
+    ENDIF (FIL STREQUAL ${TCLI_SERVICE_THRIFT} OR FIL STREQUAL "parquet.thrift" OR
+        FIL STREQUAL "ImpalaService.thrift")
   endforeach(FIL)
 
   set(${VAR} ${${VAR}} PARENT_SCOPE)
@@ -134,8 +140,8 @@ file(MAKE_DIRECTORY ${PYTHON_OUTPUT_DIR})
 file(MAKE_DIRECTORY ${HIVE_THRIFT_SOURCE_DIR})
 
 # Args passed to thrift for Java gen
-set(JAVA_FE_ARGS ${THRIFT_INCLUDE_DIR_OPTION} --gen java:hashcode -o ${FE_OUTPUT_DIR})
-set(JAVA_EXT_DS_ARGS ${THRIFT_INCLUDE_DIR_OPTION} --gen java:hashcode -o ${EXT_DS_OUTPUT_DIR})
+set(JAVA_FE_ARGS ${THRIFT_INCLUDE_DIR_OPTION} --gen java -o ${FE_OUTPUT_DIR})
+set(JAVA_EXT_DS_ARGS ${THRIFT_INCLUDE_DIR_OPTION} --gen java -o ${EXT_DS_OUTPUT_DIR})
 set(PYTHON_ARGS ${THRIFT_INCLUDE_DIR_OPTION} -r --gen py -o ${PYTHON_OUTPUT_DIR})
 
 set (EXT_DATA_SRC_FILES

fe/pom.xml

@@ -167,27 +167,6 @@ under the License.
       <groupId>org.apache.thrift</groupId>
       <artifactId>libthrift</artifactId>
       <version>${thrift.version}</version>
-      <!-- libthrift depends httpcore 4.1.3 which does not work with KMS. To workaround
-           this problem the dependency is excluded here and we explicitly add a newer
-           httpcore dependency version. See IMPALA-4210. TODO: Find a better fix. -->
-      <exclusions>
-        <exclusion>
-          <groupId>org.apache.httpcomponents</groupId>
-          <artifactId>httpcore</artifactId>
-      </exclusion>
-    </exclusions>
-    </dependency>
-
-    <dependency>
-      <groupId>org.apache.httpcomponents</groupId>
-      <artifactId>httpcore</artifactId>
-      <version>${httpcomponents.core.version}</version>
-    </dependency>
-
-    <dependency>
-      <groupId>org.apache.thrift</groupId>
-      <artifactId>libfb303</artifactId>
-      <version>${thrift.version}</version>
     </dependency>
 
     <dependency>

infra/python/deps/compiled-requirements.txt

@@ -27,9 +27,6 @@ impyla == 0.14.0
   bitarray == 0.8.1
   sasl == 0.1.3
   six == 1.11.0
-  # Thrift usually comes from the thirdparty dir but in case the virtualenv is needed
-  # before thirdparty is built thrift will be installed anyways.
-  thrift == 0.9.0
   thrift-sasl == 0.1.0
 psutil == 0.7.1
 # Required for Kudu: