You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by wz...@apache.org on 2021/11/12 18:07:30 UTC
[impala] 02/02: IMPALA-11005 (part 2): Upgrade Boost library to
1.74.0 for Impala
This is an automated email from the ASF dual-hosted git repository.
wzhou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit bf728516fe8b6f85b557db1aea7fa215f609a08f
Author: wzhou-code <wz...@cloudera.com>
AuthorDate: Sat Oct 16 11:21:53 2021 -0700
IMPALA-11005 (part 2): Upgrade Boost library to 1.74.0 for Impala
There are some header files deprecated in the new version of Boost
library. Need to define BOOST_ALLOW_DEPRECATED_HEADERS in
CMakeLists.txt to avoid compiling errors. Also define
BOOST_BIND_GLOBAL_PLACEHOLDERS to keep current behaviour of boost::bind
and avoid big code changes.
Define exception handler for a new boost::throw_exception() API since
BOOST_NO_EXCEPTIONS is defined in be/CMakeLists.txt and we have to
provide handlers which will be called by codegen'd code.
Revert the code change made by IMPALA-2846 and IMPALA-9571 since the
bug was fixed in Boost 1.74.0.
Testing:
- Passed core DEBUG build and exhaustive release build.
- Passed core ASAN build.
Change-Id: I78f32ae3c274274325e7af9e9bc9643814ae346a
Reviewed-on: http://gerrit.cloudera.org:8080/17996
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
---
be/CMakeLists.txt | 3 +++
be/src/benchmarks/convert-timestamp-benchmark.cc | 3 ++-
be/src/codegen/llvm-codegen.cc | 8 +++++++-
be/src/runtime/bufferpool/buffer-pool-test.cc | 8 ++++----
be/src/runtime/timestamp-value.cc | 4 ----
be/src/statestore/failure-detector.h | 1 +
be/src/util/blocking-queue-test.cc | 6 +++---
be/src/util/condition-variable.h | 2 +-
be/src/util/filesystem-util.cc | 26 +++++-------------------
bin/impala-config.sh | 4 ++--
10 files changed, 28 insertions(+), 37 deletions(-)
diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt
index e94ced3..09006b9 100644
--- a/be/CMakeLists.txt
+++ b/be/CMakeLists.txt
@@ -52,6 +52,8 @@ SET(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -std=c++14")
SET(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-deprecated -Wno-vla")
SET(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -DBOOST_DATE_TIME_POSIX_TIME_STD_CONFIG")
SET(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -DBOOST_SYSTEM_NO_DEPRECATED")
+SET(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -DBOOST_BIND_GLOBAL_PLACEHOLDERS")
+SET(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -DBOOST_ALLOW_DEPRECATED_HEADERS")
SET(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -B $ENV{IMPALA_TOOLCHAIN_PACKAGES_HOME}/binutils-$ENV{IMPALA_BINUTILS_VERSION}/bin/")
IF($ENV{USE_GOLD_LINKER} STREQUAL "true")
SET(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -fuse-ld=gold")
@@ -242,6 +244,7 @@ add_definitions(-DKUDU_HEADERS_USE_RICH_SLICE -DKUDU_HEADERS_NO_STUBS)
# -DBOOST_NO_EXCEPTIONS: call a custom error handler for exceptions in codegen'd code.
set(CLANG_IR_CXX_FLAGS "-emit-llvm" "-c" "-std=c++14" "-DIR_COMPILE" "-DHAVE_INTTYPES_H"
"-DHAVE_NETINET_IN_H" "-DBOOST_DATE_TIME_POSIX_TIME_STD_CONFIG" "-DBOOST_NO_EXCEPTIONS"
+ "-DBOOST_BIND_GLOBAL_PLACEHOLDERS" "-DBOOST_ALLOW_DEPRECATED_HEADERS"
"-DKUDU_HEADERS_NO_STUBS" "-fcolor-diagnostics" "-Wno-deprecated"
"-Wno-return-type-c-linkage" "-fsigned-char" "-O1")
diff --git a/be/src/benchmarks/convert-timestamp-benchmark.cc b/be/src/benchmarks/convert-timestamp-benchmark.cc
index c263fcf..84ba26c 100644
--- a/be/src/benchmarks/convert-timestamp-benchmark.cc
+++ b/be/src/benchmarks/convert-timestamp-benchmark.cc
@@ -610,7 +610,8 @@ TimestampValue from_subsecond_unix_time_old(const double& unix_time) {
const time_t unix_time_whole = unix_time;
boost::posix_time::ptime temp =
cctz_optimized_unix_time_to_utc_ptime(unix_time_whole);
- temp += boost::posix_time::nanoseconds((unix_time - unix_time_whole) / ONE_BILLIONTH);
+ int64_t nanos = (unix_time - unix_time_whole) / ONE_BILLIONTH;
+ temp += boost::posix_time::nanoseconds(nanos);
return TimestampValue(temp);
}
diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc
index 48c4f5c..94b2e57 100644
--- a/be/src/codegen/llvm-codegen.cc
+++ b/be/src/codegen/llvm-codegen.cc
@@ -23,6 +23,7 @@
#include <mutex>
#include <boost/algorithm/string.hpp>
+#include <boost/assert/source_location.hpp>
#include <gutil/strings/substitute.h>
#include <llvm/ADT/Triple.h>
@@ -1838,10 +1839,15 @@ string LlvmCodeGen::DiagnosticHandler::GetErrorString() {
namespace boost {
/// Handler for exceptions in cross-compiled functions.
-/// When boost is configured with BOOST_NO_EXCEPTIONS, it calls this handler instead of
+/// When boost is configured with BOOST_NO_EXCEPTIONS, it calls these handlers instead of
/// throwing the exception.
[[noreturn]] void throw_exception(std::exception const& e) {
LOG(FATAL) << "Cannot handle exceptions in codegen'd code " << e.what();
}
+[[noreturn]] void throw_exception(
+ std::exception const& e, boost::source_location const& loc) {
+ LOG(FATAL) << loc.file_name() << ":" << loc.line() << "] " << loc.function_name()
+ << ": Cannot handle exceptions in codegen'd code " << e.what();
+}
}
diff --git a/be/src/runtime/bufferpool/buffer-pool-test.cc b/be/src/runtime/bufferpool/buffer-pool-test.cc
index 7765ac9..b9946ba 100644
--- a/be/src/runtime/bufferpool/buffer-pool-test.cc
+++ b/be/src/runtime/bufferpool/buffer-pool-test.cc
@@ -568,8 +568,8 @@ TEST_F(BufferPoolTest, ConcurrentRegistration) {
// Launch threads, each with a different set of query IDs.
thread_group workers;
for (int i = 0; i < num_threads; ++i) {
- workers.add_thread(new thread(bind(&BufferPoolTest::RegisterQueriesAndClients, this,
- &pool, i, queries_per_thread, sum_initial_reservations, reservation_limit,
+ workers.add_thread(new thread(boost::bind(&BufferPoolTest::RegisterQueriesAndClients,
+ this, &pool, i, queries_per_thread, sum_initial_reservations, reservation_limit,
&thread_rngs[i])));
}
workers.join_all();
@@ -1145,8 +1145,8 @@ TEST_F(BufferPoolTest, ConcurrentPageCreation) {
// Launch threads, each with a different set of query IDs.
thread_group workers;
for (int i = 0; i < num_threads; ++i) {
- workers.add_thread(new thread(bind(&BufferPoolTest::CreatePageLoop, this, &pool,
- file_group, &global_reservations_, ops_per_thread)));
+ workers.add_thread(new thread(boost::bind(&BufferPoolTest::CreatePageLoop, this,
+ &pool, file_group, &global_reservations_, ops_per_thread)));
}
// Build debug string to test concurrent iteration over pages_ list.
diff --git a/be/src/runtime/timestamp-value.cc b/be/src/runtime/timestamp-value.cc
index ddedebc..26d8199 100644
--- a/be/src/runtime/timestamp-value.cc
+++ b/be/src/runtime/timestamp-value.cc
@@ -35,10 +35,6 @@ using boost::posix_time::ptime_from_tm;
using boost::posix_time::time_duration;
using boost::posix_time::to_tm;
-// Boost stores dates as an uint32_t. Since subtraction is needed, convert to signed.
-const int64_t EPOCH_DAY_NUMBER =
- static_cast<int64_t>(date(1970, boost::gregorian::Jan, 1).day_number());
-
namespace impala {
using datetime_parse_util::DateTimeFormatContext;
diff --git a/be/src/statestore/failure-detector.h b/be/src/statestore/failure-detector.h
index d44cef8..6d2700a 100644
--- a/be/src/statestore/failure-detector.h
+++ b/be/src/statestore/failure-detector.h
@@ -17,6 +17,7 @@
#pragma once
+#include <map>
#include <mutex>
#include <string>
#include <boost/date_time/posix_time/posix_time_types.hpp>
diff --git a/be/src/util/blocking-queue-test.cc b/be/src/util/blocking-queue-test.cc
index acc9e35..918e156 100644
--- a/be/src/util/blocking-queue-test.cc
+++ b/be/src/util/blocking-queue-test.cc
@@ -150,14 +150,14 @@ class MultiThreadTest { // NOLINT: members are not arranged for minimal padding
void Run() {
for (int i = 0; i < nthreads_; ++i) {
threads_.push_back(shared_ptr<thread>(
- new thread(bind(&MultiThreadTest::InserterThread, this, i))));
+ new thread(boost::bind(&MultiThreadTest::InserterThread, this, i))));
threads_.push_back(shared_ptr<thread>(
- new thread(bind(&MultiThreadTest::RemoverThread, this))));
+ new thread(boost::bind(&MultiThreadTest::RemoverThread, this))));
}
// We add an extra thread to ensure that there aren't enough elements in
// the queue to go around. This way, we test removal after Shutdown.
threads_.push_back(shared_ptr<thread>(
- new thread(bind(
+ new thread(boost::bind(
&MultiThreadTest::RemoverThread, this))));
for (int i = 0; i < threads_.size(); ++i) {
threads_[i]->join();
diff --git a/be/src/util/condition-variable.h b/be/src/util/condition-variable.h
index f12031e..ad7d885 100644
--- a/be/src/util/condition-variable.h
+++ b/be/src/util/condition-variable.h
@@ -20,7 +20,7 @@
#include <pthread.h>
#include <unistd.h>
#include <mutex>
-#include <boost/thread/pthread/timespec.hpp>
+#include <boost/thread/detail/platform_time.hpp>
#include "util/time.h"
diff --git a/be/src/util/filesystem-util.cc b/be/src/util/filesystem-util.cc
index 92ad0aa..89aa007 100644
--- a/be/src/util/filesystem-util.cc
+++ b/be/src/util/filesystem-util.cc
@@ -88,19 +88,10 @@ Status FileSystemUtil::RemoveAndCreateDirectory(const string& directory) {
// the check for existence above and the removal here. If the directory is removed in
// this window, we may get "no_such_file_or_directory" error which is fine.
//
- // There is a bug in boost library (as of version 1.61) which may lead to unexpected
- // exceptions from the no-exceptions interface. Even worse, the no-exceptions
- // interface is marked as noexcept, so the unexpected exception will cause the
- // program to immediately call std::terminate(). This renders the try/catch block
- // useless. For now, this uses the interface that can throw exceptions and
- // gets the errorcode from the exception. See IMPALA-2846 + IMPALA-9571.
- // TODO: Newer boost has a fix for this bug, so this can be switched back after
- // upgrading boost.
- try {
- filesystem::remove_all(directory);
- } catch (filesystem::filesystem_error& e) {
- errcode = e.code();
- }
+ // Since unexpected exceptions from the no-exceptions interface (Ticket #7307) was
+ // fixed in boost 1.63.0, revert the code change made by IMPALA-2846 + IMPALA-9571
+ // when upgrading boost to 1.74.0.
+ filesystem::remove_all(directory, errcode);
if (errcode != errc::success &&
errcode != errc::no_such_file_or_directory) {
return Status(ErrorMsg(TErrorCode::RUNTIME_ERROR, Substitute("Encountered error "
@@ -118,14 +109,7 @@ Status FileSystemUtil::RemoveAndCreateDirectory(const string& directory) {
Status FileSystemUtil::RemovePaths(const vector<string>& directories) {
for (int i = 0; i < directories.size(); ++i) {
error_code errcode;
- // This uses the boost remove_all() interface that can throw exceptions to avoid a
- // bug in the implementation of the no-exceptions remove_all(). See comment in
- // RemoveAndCreateDirectory() for more details.
- try {
- filesystem::remove_all(directories[i]);
- } catch (filesystem::filesystem_error& e) {
- errcode = e.code();
- }
+ filesystem::remove_all(directories[i], errcode);
if (errcode != errc::success) {
return Status(ErrorMsg(TErrorCode::RUNTIME_ERROR, Substitute(
"Encountered error removing directory $0: $1", directories[i],
diff --git a/bin/impala-config.sh b/bin/impala-config.sh
index 171fd7b..f95ef6b 100755
--- a/bin/impala-config.sh
+++ b/bin/impala-config.sh
@@ -68,14 +68,14 @@ 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=55-77e9adb973
+export IMPALA_TOOLCHAIN_BUILD_ID=58-0f013cd00a
# Versions of toolchain dependencies.
# -----------------------------------
export IMPALA_AVRO_VERSION=1.7.4-p5
unset IMPALA_AVRO_URL
export IMPALA_BINUTILS_VERSION=2.28
unset IMPALA_BINUTILS_URL
-export IMPALA_BOOST_VERSION=1.61.0-p2
+export IMPALA_BOOST_VERSION=1.74.0-p1
unset IMPALA_BOOST_URL
export IMPALA_BREAKPAD_VERSION=97a98836768f8f0154f8f86e5e14c2bb7e74132e-p2
unset IMPALA_BREAKPAD_URL