Skip to content

Commit

Permalink
Fix TSAN data race flaky test
Browse files Browse the repository at this point in the history
Summary: There were some race conditions that I'm fixing.

Differential Revision: D55817149
  • Loading branch information
Daniel Munoz authored and facebook-github-bot committed Apr 5, 2024
1 parent 41bed84 commit 8cab9f5
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 9 deletions.
2 changes: 1 addition & 1 deletion velox/dwio/common/tests/utils/UnitLoaderTestTools.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ReaderMock::ReaderMock(
UnitLoaderFactory& factory)
: rowsPerUnit_{std::move(rowsPerUnit)},
ioSizes_{std::move(ioSizes)},
unitsLoaded_(rowsPerUnit_.size(), false),
unitsLoaded_(std::vector<bool>(rowsPerUnit_.size())),
loader_{factory.create(getUnits())} {
VELOX_CHECK(rowsPerUnit_.size() == ioSizes_.size());
}
Expand Down
17 changes: 9 additions & 8 deletions velox/dwio/common/tests/utils/UnitLoaderTestTools.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <memory>
#include <vector>

#include "folly/Synchronized.h"
#include "velox/common/base/Exceptions.h"
#include "velox/dwio/common/UnitLoader.h"

Expand All @@ -16,7 +17,7 @@ class LoadUnitMock : public LoadUnit {
LoadUnitMock(
uint64_t rowCount,
uint64_t ioSize,
std::vector<bool>& unitsLoaded,
folly::Synchronized<std::vector<bool>>& unitsLoaded,
size_t unitId)
: rowCount_{rowCount},
ioSize_{ioSize},
Expand All @@ -27,12 +28,12 @@ class LoadUnitMock : public LoadUnit {

void load() override {
VELOX_CHECK(!isLoaded());
unitsLoaded_[unitId_] = true;
(*unitsLoaded_.wlock())[unitId_] = true;
}

void unload() override {
VELOX_CHECK(isLoaded());
unitsLoaded_[unitId_] = false;
(*unitsLoaded_.wlock())[unitId_] = false;
}

uint64_t getNumRows() override {
Expand All @@ -44,13 +45,13 @@ class LoadUnitMock : public LoadUnit {
}

bool isLoaded() const {
return unitsLoaded_[unitId_];
return (*unitsLoaded_.rlock())[unitId_];
}

private:
uint64_t rowCount_;
uint64_t ioSize_;
std::vector<bool>& unitsLoaded_;
folly::Synchronized<std::vector<bool>>& unitsLoaded_;
size_t unitId_;
};

Expand All @@ -63,8 +64,8 @@ class ReaderMock {

bool read(uint64_t maxRows);

const std::vector<bool>& unitsLoaded() const {
return unitsLoaded_;
std::vector<bool> unitsLoaded() const {
return unitsLoaded_.withRLock([](auto unitsLoaded) { return unitsLoaded; });
}

private:
Expand All @@ -74,7 +75,7 @@ class ReaderMock {

std::vector<uint64_t> rowsPerUnit_;
std::vector<uint64_t> ioSizes_;
std::vector<bool> unitsLoaded_;
folly::Synchronized<std::vector<bool>> unitsLoaded_;
std::unique_ptr<UnitLoader> loader_;
size_t currentUnit_{0};
size_t currentRowInUnit_{0};
Expand Down

0 comments on commit 8cab9f5

Please sign in to comment.