This is an automated email from the ASF dual-hosted git repository. stigahuang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 8d05f5134cc95f53e4e4bbd8ceb9de88b845fda1 Author: Joe McDonnell <[email protected]> AuthorDate: Mon Jun 10 20:09:20 2024 -0700 IMPALA-13128: disk-file-test Hangs on ARM + UBSAN Test Jobs The Jenkins jobs that run the UBSAN tests on ARM were occaisonally hanging on the disk-file-test. This commit fixes these hangs by upgrading Google Test and implementing the Death Test handling functionality which safely runs tests that expect the process to die. See https://github.com/google/googletest/blob/main/docs/advanced.md#death-tests for details on known problems with running death tests and threads at the same time causing tests to hang. Testing was accomplished by running the disk-file-test repeatedly in a loop on a RHEL 8.9 ARM machine. Before this fix was implemented, this test would run up to 70 times before it hung. After the fix was implemented, the test ran 2,490 times and was still running when it was stopped. These test runs had durations between 18.7 and 19.9 seconds which means disk-file-test now takes about 15 seconds longer than its previous duration of about 4.4 seconds. Change-Id: Ie01f7781f24644a66e9ec52652450116f5cb4297 Reviewed-on: http://gerrit.cloudera.org:8080/21544 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- CMakeLists.txt | 3 ++- be/src/codegen/llvm-codegen-test.cc | 2 +- be/src/exec/json/json-parser-test.cc | 2 +- be/src/exprs/expr-test.cc | 2 +- be/src/rpc/rpc-mgr-kerberized-test.cc | 2 +- be/src/rpc/rpc-mgr-test.cc | 2 +- be/src/rpc/thrift-server-test.cc | 8 ++++---- be/src/runtime/io/CMakeLists.txt | 2 +- be/src/runtime/io/data-cache-test.cc | 2 +- be/src/runtime/io/disk-file-test.cc | 8 +++++++- be/src/util/cache/cache-bench.cc | 2 +- be/src/util/cache/cache-test.cc | 2 +- be/src/util/cache/rl-cache-test.cc | 4 ++-- be/src/util/runtime-profile-test.cc | 2 +- bin/bootstrap_toolchain.py | 6 ++++-- bin/impala-config.sh | 8 ++++---- 16 files changed, 33 insertions(+), 24 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9dcfeee89..0b34fb43d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -120,7 +120,8 @@ set_dep_root(GCC) set_dep_root(GFLAGS) set_dep_root(GLOG) set_dep_root(GPERFTOOLS) -set_dep_root(GTEST) +set(GTEST_ROOT + $ENV{IMPALA_TOOLCHAIN_PACKAGES_HOME}/googletest-$ENV{IMPALA_GTEST_VERSION}) set_dep_root(JWT_CPP) set_dep_root(LIBEV) set_dep_root(LIBUNWIND) diff --git a/be/src/codegen/llvm-codegen-test.cc b/be/src/codegen/llvm-codegen-test.cc index 3f6f70daa..8fb707488 100644 --- a/be/src/codegen/llvm-codegen-test.cc +++ b/be/src/codegen/llvm-codegen-test.cc @@ -704,7 +704,7 @@ TEST_P(LlvmOptTest, CachedOptFunction) { EXPECT_EQ(cache_misses->GetValue(), opt_levels.size() - num_less); } -INSTANTIATE_TEST_CASE_P(OptLevels, LlvmOptTest, ::testing::Values( +INSTANTIATE_TEST_SUITE_P(OptLevels, LlvmOptTest, ::testing::Values( // Optimization level Expect unoptimized std::make_tuple(TCodeGenOptLevel::O0, true), std::make_tuple(TCodeGenOptLevel::O1, false), diff --git a/be/src/exec/json/json-parser-test.cc b/be/src/exec/json/json-parser-test.cc index 576403e34..e2693111c 100644 --- a/be/src/exec/json/json-parser-test.cc +++ b/be/src/exec/json/json-parser-test.cc @@ -107,7 +107,7 @@ class JsonParserTest : public ::testing::TestWithParam<int> { }; }; -INSTANTIATE_TEST_CASE_P(StreamSize, JsonParserTest, ::testing::Values(1, 16, 256)); +INSTANTIATE_TEST_SUITE_P(StreamSize, JsonParserTest, ::testing::Values(1, 16, 256)); TEST_P(JsonParserTest, Basic) { SimpleJsonScanner js(schema(), [this](const char** begin, const char** end) { diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc index e57dbeeb7..63d0e9306 100644 --- a/be/src/exprs/expr-test.cc +++ b/be/src/exprs/expr-test.cc @@ -11427,7 +11427,7 @@ TEST_P(ExprTest, AiFunctionsTest) { } // namespace impala -INSTANTIATE_TEST_CASE_P(Instantiations, ExprTest, ::testing::Values( +INSTANTIATE_TEST_SUITE_P(Instantiations, ExprTest, ::testing::Values( // disable_codegen enable_expr_rewrites std::make_tuple(true, false), std::make_tuple(false, false), diff --git a/be/src/rpc/rpc-mgr-kerberized-test.cc b/be/src/rpc/rpc-mgr-kerberized-test.cc index b01e81492..e5b343f4c 100644 --- a/be/src/rpc/rpc-mgr-kerberized-test.cc +++ b/be/src/rpc/rpc-mgr-kerberized-test.cc @@ -284,7 +284,7 @@ TEST_P(RpcMgrKerberizedTest, InternalAuthorizationSkip) { // Run tests with Unix domain socket and TCP socket by setting // FLAGS_rpc_use_unix_domain_socket as true and false. -INSTANTIATE_TEST_CASE_P( +INSTANTIATE_TEST_SUITE_P( UdsOnAndOff, RpcMgrKerberizedTest, ::testing::Values(true, false)); } // namespace impala diff --git a/be/src/rpc/rpc-mgr-test.cc b/be/src/rpc/rpc-mgr-test.cc index 792eefac1..eb5869a7e 100644 --- a/be/src/rpc/rpc-mgr-test.cc +++ b/be/src/rpc/rpc-mgr-test.cc @@ -416,7 +416,7 @@ TEST_P(RpcMgrTest, BusyService) { // Run tests with Unix domain socket and TCP socket by setting // FLAGS_rpc_use_unix_domain_socket as true and false. -INSTANTIATE_TEST_CASE_P(UdsOnAndOff, RpcMgrTest, ::testing::Values(true, false)); +INSTANTIATE_TEST_SUITE_P(UdsOnAndOff, RpcMgrTest, ::testing::Values(true, false)); } // namespace impala diff --git a/be/src/rpc/thrift-server-test.cc b/be/src/rpc/thrift-server-test.cc index cbbb6d074..31b2cef43 100644 --- a/be/src/rpc/thrift-server-test.cc +++ b/be/src/rpc/thrift-server-test.cc @@ -135,10 +135,10 @@ class ThriftKerberizedParamsTest : } }; -INSTANTIATE_TEST_CASE_P(KerberosOnAndOff, - ThriftKerberizedParamsTest, - ::testing::Values(KERBEROS_OFF, - KERBEROS_ON)); +INSTANTIATE_TEST_SUITE_P(KerberosOnAndOff, + ThriftKerberizedParamsTest, + ::testing::Values(KERBEROS_OFF, + KERBEROS_ON)); TEST(ThriftTestBase, Connectivity) { int port = GetServerPort(); diff --git a/be/src/runtime/io/CMakeLists.txt b/be/src/runtime/io/CMakeLists.txt index 1a927e484..b3a5bd78a 100644 --- a/be/src/runtime/io/CMakeLists.txt +++ b/be/src/runtime/io/CMakeLists.txt @@ -60,7 +60,7 @@ add_executable(data-cache-trace-replayer data-cache-trace-replayer.cc) target_link_libraries(data-cache-trace-replayer ${IMPALA_TEST_LINK_LIBS}) ADD_UNIFIED_BE_LSAN_TEST(disk-io-mgr-test DiskIoMgrTest.*) -ADD_UNIFIED_BE_LSAN_TEST(disk-file-test DiskFileTest.*) +ADD_UNIFIED_BE_LSAN_TEST(disk-file-test DiskFileTest.*:DiskFileDeathTest.*) ADD_UNIFIED_BE_LSAN_TEST(data-cache-trace-test DataCacheTraceTest.*) # Exception to unified be: Custom main function (platform tests) ADD_BE_LSAN_TEST(data-cache-test) diff --git a/be/src/runtime/io/data-cache-test.cc b/be/src/runtime/io/data-cache-test.cc index 2303d3146..958dbef25 100644 --- a/be/src/runtime/io/data-cache-test.cc +++ b/be/src/runtime/io/data-cache-test.cc @@ -332,7 +332,7 @@ class DataCacheTest : } }; -INSTANTIATE_TEST_CASE_P(DataCacheTestTypes, DataCacheTest,::testing::Values( +INSTANTIATE_TEST_SUITE_P(DataCacheTestTypes, DataCacheTest,::testing::Values( DataCacheTestParam("LRU", 0), DataCacheTestParam("LRU", NUM_THREADS), DataCacheTestParam("LIRS", 0), diff --git a/be/src/runtime/io/disk-file-test.cc b/be/src/runtime/io/disk-file-test.cc index 5dc917dae..0dde9d020 100644 --- a/be/src/runtime/io/disk-file-test.cc +++ b/be/src/runtime/io/disk-file-test.cc @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +#include <gtest/gtest.h> + #include "common/names.h" #include "runtime/io/disk-io-mgr-internal.h" #include "testutil/death-test-util.h" @@ -30,6 +32,8 @@ class DiskFileTest : public testing::Test { MemBlockStatus new_status, bool expect_success); }; +typedef DiskFileTest DiskFileDeathTest; + // last_status is the MemBlock's last status it is going to reach other than // MemBlockStatus::DISABLED. void DiskFileTest::ValidateMemBlockStatus(MemBlockStatus last_status) { @@ -88,7 +92,9 @@ TEST_F(DiskFileTest, MemBlockTest) { } // Test the MemBlock status transition. -TEST_F(DiskFileTest, MemBlockStatusTransition) { +TEST_F(DiskFileDeathTest, MemBlockStatusTransition) { + GTEST_FLAG_SET(death_test_style, "threadsafe"); + MemBlock block(0); ValidateMemBlockStatusTransition( block, MemBlockStatus::UNINIT, MemBlockStatus::UNINIT, false); diff --git a/be/src/util/cache/cache-bench.cc b/be/src/util/cache/cache-bench.cc index bf2e162f8..2077d05f7 100644 --- a/be/src/util/cache/cache-bench.cc +++ b/be/src/util/cache/cache-bench.cc @@ -163,7 +163,7 @@ class CacheBench : public testing::Test, // Test both distributions, and for each, test both the case where the data // fits in the cache and where it is a bit larger. -INSTANTIATE_TEST_CASE_P(Patterns, CacheBench, testing::ValuesIn(std::vector<BenchSetup>{ +INSTANTIATE_TEST_SUITE_P(Patterns, CacheBench, testing::ValuesIn(std::vector<BenchSetup>{ {BenchSetup::Pattern::ZIPFIAN, 1.0}, {BenchSetup::Pattern::ZIPFIAN, 3.0}, {BenchSetup::Pattern::UNIFORM, 1.0}, diff --git a/be/src/util/cache/cache-test.cc b/be/src/util/cache/cache-test.cc index 2eebdec44..17aabe04b 100644 --- a/be/src/util/cache/cache-test.cc +++ b/be/src/util/cache/cache-test.cc @@ -73,7 +73,7 @@ class CacheTest : } }; -INSTANTIATE_TEST_CASE_P( +INSTANTIATE_TEST_SUITE_P( CacheTypes, CacheTest, ::testing::Values( make_tuple(Cache::EvictionPolicy::FIFO, diff --git a/be/src/util/cache/rl-cache-test.cc b/be/src/util/cache/rl-cache-test.cc index 75cf3d01d..ff27bd15c 100644 --- a/be/src/util/cache/rl-cache-test.cc +++ b/be/src/util/cache/rl-cache-test.cc @@ -41,7 +41,7 @@ class CacheInvalidationTest : } }; -INSTANTIATE_TEST_CASE_P( +INSTANTIATE_TEST_SUITE_P( CacheTypes, CacheInvalidationTest, ::testing::Values( make_tuple(Cache::EvictionPolicy::FIFO, @@ -277,7 +277,7 @@ class LRUCacheTest : } }; -INSTANTIATE_TEST_CASE_P( +INSTANTIATE_TEST_SUITE_P( CacheTypes, LRUCacheTest, ::testing::Values(ShardingPolicy::MultiShard, ShardingPolicy::SingleShard)); diff --git a/be/src/util/runtime-profile-test.cc b/be/src/util/runtime-profile-test.cc index 718286b03..0ef2893f8 100644 --- a/be/src/util/runtime-profile-test.cc +++ b/be/src/util/runtime-profile-test.cc @@ -1661,7 +1661,7 @@ TEST_P(TimeSeriesCounterResampleTest, TestPrettyPrint) { for (const char* e : param.expected) EXPECT_STR_CONTAINS(pretty_str, e); } -INSTANTIATE_TEST_CASE_P(VariousNumbers, TimeSeriesCounterResampleTest, +INSTANTIATE_TEST_SUITE_P(VariousNumbers, TimeSeriesCounterResampleTest, ::testing::Values( TimeSeriesTestParam(64, {"TestCounter (500.000ms): 0, 1, 2, 3", "61, 62, 63"}), diff --git a/bin/bootstrap_toolchain.py b/bin/bootstrap_toolchain.py index 670e2f447..fea6a2ed3 100755 --- a/bin/bootstrap_toolchain.py +++ b/bin/bootstrap_toolchain.py @@ -474,12 +474,14 @@ def get_toolchain_downloads(): toolchain_packages += [ToolchainPackage(p) for p in ["avro", "binutils", "boost", "breakpad", "bzip2", "calloncehack", "cctz", "cloudflarezlib", "cmake", "crcutil", "curl", "flatbuffers", "gdb", "gflags", - "glog", "gperftools", "gtest", "jwt-cpp", "libev", "libunwind", "lz4", "mold", + "glog", "gperftools", "jwt-cpp", "libev", "libunwind", "lz4", "mold", "openldap", "orc", "protobuf", "python", "rapidjson", "re2", "snappy", "tpc-h", "tpc-ds", "zlib", "zstd"]] python3_package = ToolchainPackage( "python", explicit_version=os.environ.get("IMPALA_PYTHON3_VERSION")) - toolchain_packages += [python3_package] + gtest_package = ToolchainPackage( + "googletest", explicit_version=os.environ.get("IMPALA_GTEST_VERSION")) + toolchain_packages += [python3_package, gtest_package] toolchain_packages += get_unique_toolchain_downloads( ["thrift:cpp", "thrift:java", "thrift:py"]) protobuf_package_clang = ToolchainPackage( diff --git a/bin/impala-config.sh b/bin/impala-config.sh index eb4f3a1d9..7e70a58cb 100755 --- a/bin/impala-config.sh +++ b/bin/impala-config.sh @@ -85,13 +85,13 @@ export USE_AVRO_CPP=${USE_AVRO_CPP:=false} # 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_AARCH64=34-f93e2c9a86 -export IMPALA_TOOLCHAIN_BUILD_ID_X86_64=422-f93e2c9a86 +export IMPALA_TOOLCHAIN_BUILD_ID_AARCH64=45-e78b0ef342 +export IMPALA_TOOLCHAIN_BUILD_ID_X86_64=438-e78b0ef342 export IMPALA_TOOLCHAIN_REPO=\ ${IMPALA_TOOLCHAIN_REPO:-https://github.com/cloudera/native-toolchain.git} export IMPALA_TOOLCHAIN_BRANCH=${IMPALA_TOOLCHAIN_BRANCH:-master} export IMPALA_TOOLCHAIN_COMMIT_HASH=\ -${IMPALA_TOOLCHAIN_COMMIT_HASH-f93e2c9a865c80cafd76b872ad04400877766a2f} +${IMPALA_TOOLCHAIN_COMMIT_HASH-e78b0ef34241218cda7eac3b526cb6a824596df1} # Compare the build ref in build IDs by removing everything up-to-and-including the # first hyphen. if [ "${IMPALA_TOOLCHAIN_BUILD_ID_AARCH64#*-}" \ @@ -141,7 +141,7 @@ export IMPALA_GLOG_VERSION=0.3.5-p3 unset IMPALA_GLOG_URL export IMPALA_GPERFTOOLS_VERSION=2.10 unset IMPALA_GPERFTOOLS_URL -export IMPALA_GTEST_VERSION=1.6.0 +export IMPALA_GTEST_VERSION=1.14.0 unset IMPALA_GTEST_URL export IMPALA_JWT_CPP_VERSION=0.5.0 unset IMPALA_JWT_CPP_URL
