Skip to content

Commit

Permalink
Fix TSAN for PeriodicStatsReporterTest (facebookincubator#9692)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebookincubator#9692

TSAN complains as 2 concurrently running threads are trying to access the same memory location. Stop the reporter early to let TSAN not complain

Reviewed By: xiaoxmeng

Differential Revision: D56891040

fbshipit-source-id: 14403415b3c5399f73dafc08332a7e2858d0532e
  • Loading branch information
Jialiang Tan authored and facebook-github-bot committed May 2, 2024
1 parent d7c4f88 commit 7fa9fd2
Showing 1 changed file with 7 additions and 5 deletions.
12 changes: 7 additions & 5 deletions velox/common/base/tests/StatsReporterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ TEST_F(StatsReporterTest, trivialReporter) {
EXPECT_EQ(100, reporter_->counterMap["key4"]);
};

class PeriodicStatsReportDaemonTest : public StatsReporterTest {};
class PeriodicStatsReporterTest : public StatsReporterTest {};

class TestStatsReportMemoryArbitrator : public memory::MemoryArbitrator {
public:
Expand Down Expand Up @@ -193,21 +193,23 @@ class TestStatsReportMemoryArbitrator : public memory::MemoryArbitrator {
memory::MemoryArbitrator::Stats stats_;
};

TEST_F(PeriodicStatsReportDaemonTest, basic) {
TEST_F(PeriodicStatsReporterTest, basic) {
TestStatsReportMemoryArbitrator arbitrator({});
PeriodicStatsReporter::Options options;
options.arbitratorStatsIntervalMs = 4'000;
PeriodicStatsReporter reporter(&arbitrator, options);
PeriodicStatsReporter periodicReporter(&arbitrator, options);

reporter.start();
periodicReporter.start();
std::this_thread::sleep_for(std::chrono::milliseconds(2'000));
// Stop right after sufficient wait to ensure the following reads from main
// thread does not trigger TSAN failures.
periodicReporter.stop();

const auto& counterMap = reporter_->counterMap;
ASSERT_EQ(counterMap.size(), 2);
ASSERT_EQ(counterMap.count(kMetricArbitratorFreeCapacityBytes.str()), 1);
ASSERT_EQ(
counterMap.count(kMetricArbitratorFreeReservedCapacityBytes.str()), 1);
reporter.stop();
}

// Registering to folly Singleton with intended reporter type
Expand Down

0 comments on commit 7fa9fd2

Please sign in to comment.