Skip to content

Commit

Permalink
Fix flaky test in PeriodicStatsReport tests (facebookincubator#9846)
Browse files Browse the repository at this point in the history
Summary:

The test reporter's map is not protected by lock, causing race conditions when counterMap is modified. This made the test flaky

Differential Revision: D57469593
  • Loading branch information
Jialiang Tan authored and facebook-github-bot committed May 16, 2024
1 parent 4bde4b0 commit 811f2ec
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions velox/common/base/tests/StatsReporterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ namespace facebook::velox {

class TestReporter : public BaseStatsReporter {
public:
mutable std::mutex m;
mutable std::unordered_map<std::string, size_t> counterMap;
mutable std::unordered_map<std::string, StatType> statTypeMap;
mutable std::unordered_map<std::string, std::vector<int32_t>>
histogramPercentilesMap;

void clear() {
std::lock_guard<std::mutex> l(m);
counterMap.clear();
statTypeMap.clear();
histogramPercentilesMap.clear();
Expand Down Expand Up @@ -74,28 +76,34 @@ class TestReporter : public BaseStatsReporter {

void addMetricValue(const std::string& key, const size_t value)
const override {
std::lock_guard<std::mutex> l(m);
counterMap[key] += value;
}

void addMetricValue(const char* key, const size_t value) const override {
std::lock_guard<std::mutex> l(m);
counterMap[key] += value;
}

void addMetricValue(folly::StringPiece key, size_t value) const override {
std::lock_guard<std::mutex> l(m);
counterMap[key.str()] += value;
}

void addHistogramMetricValue(const std::string& key, size_t value)
const override {
std::lock_guard<std::mutex> l(m);
counterMap[key] = std::max(counterMap[key], value);
}

void addHistogramMetricValue(const char* key, size_t value) const override {
std::lock_guard<std::mutex> l(m);
counterMap[key] = std::max(counterMap[key], value);
}

void addHistogramMetricValue(folly::StringPiece key, size_t value)
const override {
std::lock_guard<std::mutex> l(m);
counterMap[key.str()] = std::max(counterMap[key.str()], value);
}
};
Expand Down

0 comments on commit 811f2ec

Please sign in to comment.