Skip to content

Commit

Permalink
Make exceededMaxSpillLevelLimit_ in HashBuild and HashProbe tsan_atom…
Browse files Browse the repository at this point in the history
…ic (#11165)

Summary:
Pull Request resolved: #11165

setupSpiller and canReclaim may be called concurrently in HashBuild and HashProbe
which leads to TSAN errors in our tests as the write to and read from
exceededMaxSpillLevelLimit_ respectively.

In practice reclaim checks for the case where exceededMaxSpillLevelLimit_ has been
updated since the last time canReclaim has been called and handles that case, so this
will not cause issues.

Marking exceededMaxSpillLevelLimit_ tsan_atomic to get the tests green.

Reviewed By: xiaoxmeng, tanjialiang

Differential Revision: D63853455

fbshipit-source-id: a633f92fe521004b7b99a26b8fea29647b7e29c3
  • Loading branch information
Kevin Wilfong authored and facebook-github-bot committed Oct 4, 2024
1 parent a68114a commit ad7d0cf
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion velox/exec/HashBuild.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ class HashBuild final : public Operator {

std::shared_ptr<HashJoinBridge> joinBridge_;

bool exceededMaxSpillLevelLimit_{false};
tsan_atomic<bool> exceededMaxSpillLevelLimit_{false};

State state_{State::kRunning};

Expand Down
2 changes: 1 addition & 1 deletion velox/exec/HashProbe.h
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ class HashProbe : public Operator {
// Indicates if this hash probe has exceeded max spill limit which is not
// allowed to spill. This is reset when hash probe operator starts to probe
// the next previously spilled hash table partition.
bool exceededMaxSpillLevelLimit_{false};
tsan_atomic<bool> exceededMaxSpillLevelLimit_{false};

// The partition bits used to spill the hash table.
HashBitRange tableSpillHashBits_;
Expand Down

0 comments on commit ad7d0cf

Please sign in to comment.