You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2023/12/17 22:39:23 UTC
(impala) 03/03: IMPALA-12632: Use Atomics for CpuUsageRatio counters
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
commit 4114fe8db6ec80b2e1679e946555f91ab7043f2e
Author: Andrew Sherman <as...@cloudera.com>
AuthorDate: Thu Dec 14 10:26:57 2023 -0800
IMPALA-12632: Use Atomics for CpuUsageRatio counters
Now that IMPALA-12614 is fixed we see another data race in TSAN builds.
Fix this by using the same strategy as IMPALA-12614, and use
AtomicInt32 for the CpuUsageRatios values.
Testing: TSAN tests cover this case.
Change-Id: I373cb8ae1a45e5ec07318ccb5870e65efc906cca
Reviewed-on: http://gerrit.cloudera.org:8080/20798
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/runtime/query-state.cc | 6 +++---
be/src/util/system-state-info-test.cc | 16 ++++++++--------
be/src/util/system-state-info.cc | 8 +++++---
be/src/util/system-state-info.h | 6 +++---
4 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/be/src/runtime/query-state.cc b/be/src/runtime/query-state.cc
index dafc90b3d..4417aa589 100644
--- a/be/src/runtime/query-state.cc
+++ b/be/src/runtime/query-state.cc
@@ -220,15 +220,15 @@ Status QueryState::Init(const ExecQueryFInstancesRequestPB* exec_rpc_params,
SystemStateInfo* system_state_info = exec_env->system_state_info();
host_profile_->AddSamplingTimeSeriesCounter(
"HostCpuUserPercentage", TUnit::BASIS_POINTS, [system_state_info] () {
- return system_state_info->GetCpuUsageRatios().user;
+ return system_state_info->GetCpuUsageRatios().user.Load();
});
host_profile_->AddSamplingTimeSeriesCounter(
"HostCpuSysPercentage", TUnit::BASIS_POINTS, [system_state_info] () {
- return system_state_info->GetCpuUsageRatios().system;
+ return system_state_info->GetCpuUsageRatios().system.Load();
});
host_profile_->AddSamplingTimeSeriesCounter(
"HostCpuIoWaitPercentage", TUnit::BASIS_POINTS, [system_state_info] () {
- return system_state_info->GetCpuUsageRatios().iowait;
+ return system_state_info->GetCpuUsageRatios().iowait.Load();
});
// Add network usage
host_profile_->AddSamplingTimeSeriesCounter(
diff --git a/be/src/util/system-state-info-test.cc b/be/src/util/system-state-info-test.cc
index 78735831f..f639ea36e 100644
--- a/be/src/util/system-state-info-test.cc
+++ b/be/src/util/system-state-info-test.cc
@@ -32,7 +32,7 @@ class SystemStateInfoTest : public testing::Test {
TEST_F(SystemStateInfoTest, FirstCallReturnsZero) {
const SystemStateInfo::CpuUsageRatios& r = info.GetCpuUsageRatios();
- EXPECT_EQ(0, r.user + r.system + r.iowait);
+ EXPECT_EQ(0, r.user.Load() + r.system.Load() + r.iowait.Load());
const SystemStateInfo::NetworkUsage& n = info.GetNetworkUsage();
EXPECT_EQ(0, n.rx_rate.Load() + n.tx_rate.Load());
@@ -144,9 +144,9 @@ TEST_F(SystemStateInfoTest, ComputeCpuRatiosIntOverflow) {
info.ReadProcStatString("cpu 100953598 534 18552882 6318826150 4119619 0 0 0 0 0");
info.ComputeCpuRatios();
const SystemStateInfo::CpuUsageRatios& r = info.GetCpuUsageRatios();
- EXPECT_EQ(r.user, 1649);
- EXPECT_EQ(r.system, 304);
- EXPECT_EQ(r.iowait, 0);
+ EXPECT_EQ(r.user.Load(), 1649);
+ EXPECT_EQ(r.system.Load(), 304);
+ EXPECT_EQ(r.iowait.Load(), 0);
}
// Smoke test for the public interface.
@@ -158,7 +158,7 @@ TEST_F(SystemStateInfoTest, GetCpuUsageRatios) {
SleepForMs(200);
info.CaptureSystemStateSnapshot();
const SystemStateInfo::CpuUsageRatios& r = info.GetCpuUsageRatios();
- EXPECT_GT(r.user + r.system + r.iowait, 0);
+ EXPECT_GT(r.user.Load() + r.system.Load() + r.iowait.Load(), 0);
}
running.Store(false);
t.join();
@@ -170,9 +170,9 @@ TEST_F(SystemStateInfoTest, ComputeCpuRatios) {
info.ReadProcStatString("cpu 30 30 20 70 100 0 0 0 0 0");
info.ComputeCpuRatios();
const SystemStateInfo::CpuUsageRatios& r = info.GetCpuUsageRatios();
- EXPECT_EQ(r.user, 5000);
- EXPECT_EQ(r.system, 5000);
- EXPECT_EQ(r.iowait, 0);
+ EXPECT_EQ(r.user.Load(), 5000);
+ EXPECT_EQ(r.system.Load(), 5000);
+ EXPECT_EQ(r.iowait.Load(), 0);
}
// Tests the computation logic for network usage.
diff --git a/be/src/util/system-state-info.cc b/be/src/util/system-state-info.cc
index 931717d28..387812d36 100644
--- a/be/src/util/system-state-info.cc
+++ b/be/src/util/system-state-info.cc
@@ -146,9 +146,11 @@ void SystemStateInfo::ComputeCpuRatios() {
DCHECK_GT(total_tics, 0);
// Convert each ratio to basis points.
const int BASIS_MAX = 10000;
- cpu_ratios_.user = ((cur[CPU_USER] - old[CPU_USER]) * BASIS_MAX) / total_tics;
- cpu_ratios_.system = ((cur[CPU_SYSTEM] - old[CPU_SYSTEM]) * BASIS_MAX) / total_tics;
- cpu_ratios_.iowait = ((cur[CPU_IOWAIT] - old[CPU_IOWAIT]) * BASIS_MAX) / total_tics;
+ cpu_ratios_.user.Store(((cur[CPU_USER] - old[CPU_USER]) * BASIS_MAX) / total_tics);
+ cpu_ratios_.system.Store(
+ ((cur[CPU_SYSTEM] - old[CPU_SYSTEM]) * BASIS_MAX) / total_tics);
+ cpu_ratios_.iowait.Store(
+ ((cur[CPU_IOWAIT] - old[CPU_IOWAIT]) * BASIS_MAX) / total_tics);
}
void SystemStateInfo::ReadCurrentProcNetDev() {
diff --git a/be/src/util/system-state-info.h b/be/src/util/system-state-info.h
index fbdddef1c..ad03bc7b6 100644
--- a/be/src/util/system-state-info.h
+++ b/be/src/util/system-state-info.h
@@ -47,9 +47,9 @@ class SystemStateInfo {
/// Ratios use basis points as their unit (1/100th of a percent, i.e. 0.0001).
struct CpuUsageRatios {
- int32_t user;
- int32_t system;
- int32_t iowait;
+ AtomicInt32 user;
+ AtomicInt32 system;
+ AtomicInt32 iowait;
};
/// Returns a struct containing the CPU usage ratios for the interval between the last