This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 78d1c2cd3 IMPALA-14049: Fix TSAN issue with HdrHistogram in expr-test
78d1c2cd3 is described below
commit 78d1c2cd3a4f2c81a9142bc80b8bba9d2e9ff292
Author: Joe McDonnell <[email protected]>
AuthorDate: Wed May 7 15:46:14 2025 -0700
IMPALA-14049: Fix TSAN issue with HdrHistogram in expr-test
IMPALA-13978 switched HdrHistrogram from using a gscoped_ptr
to unique_ptr. This has been causing TSAN issues during the
teardown for expr-test. gscoped_ptr doesn't null out the
pointer when it gets destructed, but unique_ptr does. This
is a data race with the threads that are still running and
trying to access the metrics.
The full solution would be to have an orderly shutdown of
all the threads before destructing things. That is a large
project that would touch many different components. As a
short-term fix, this avoids the TSAN issue by leaking
the statestore metrics.
We should consider fixing IMPALA-9314 and implementing
orderly shutdown.
Testing:
- Ran expr-test in a loop with TSAN and didn't see this
particular issue. There are other shutdown issues with
much lower frequency that have different symptoms.
Change-Id: I73c3f4db16c6ffa272f2512e9871db5743be7a54
Reviewed-on: http://gerrit.cloudera.org:8080/22900
Reviewed-by: Riza Suminto <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
---
be/src/exprs/expr-test.cc | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index e0a117a96..67f31efb4 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -103,7 +103,7 @@ namespace impala {
const char* TEST_TZ_WITHOUT_DST = "America/Anguilla";
scoped_ptr<ImpaladQueryExecutor> executor_;
-scoped_ptr<MetricGroup> statestore_metrics(new
MetricGroup("statestore_metrics"));
+MetricGroup* statestore_metrics;
Statestore* statestore;
template <typename ORIGINAL_TYPE, typename VAL_TYPE>
@@ -222,12 +222,19 @@ class ExprTest : public
testing::TestWithParam<std::tuple<bool, bool>> {
FLAGS_disable_optimization_passes = true;
}
+ // Since the statestore is leaked and threads can be running at exit, it is
+ // unsafe to destroy the statestore_metrics before exit. Running threads
can
+ // try to access metrics and run into TSAN issues or access memory that has
+ // been freed.
+ statestore_metrics = new MetricGroup("statestore_metrics");
+ IGNORE_LEAKING_OBJECT(statestore_metrics);
+
// Create an in-process Impala server and in-process backends for test
environment
// without any startup validation check
FLAGS_abort_on_config_error = false;
VLOG_CONNECTION << "creating test env";
VLOG_CONNECTION << "starting backends";
- statestore = new Statestore(statestore_metrics.get());
+ statestore = new Statestore(statestore_metrics);
IGNORE_LEAKING_OBJECT(statestore);
// Pass in 0 to have the statestore use an ephemeral port for the service.
