Skip to content

Commit

Permalink
fix: Fix flaky tsan failure in async data cache (#11908)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #11908

Mark refresh stats method to avoid tsan check to see if it can prevent tsan failure. The refresh stats will access the
entries which are under initialization and it is read only has not pointer chasing so it is safe to do so.

Reviewed By: kagamiori, zacw7

Differential Revision: D67378594

fbshipit-source-id: 38cae1d9204880639da95c8ae9a7bcf37e471262
  • Loading branch information
xiaoxmeng authored and facebook-github-bot committed Dec 19, 2024
1 parent 4ffaf9c commit cf2ba83
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
6 changes: 5 additions & 1 deletion velox/common/caching/AsyncDataCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,9 @@ void CacheShard::updateStats(CacheStats& stats) {
if (!entry || !entry->key_.fileNum.hasValue()) {
++stats.numEmptyEntries;
continue;
} else if (entry->isExclusive()) {
}
if (entry->isExclusive()) {
stats.exclusivePinnedBytes +=
entry->data().byteSize() + entry->tinyData_.capacity();
++stats.numExclusive;
Expand All @@ -527,10 +529,12 @@ void CacheShard::updateStats(CacheStats& stats) {
entry->data().byteSize() + entry->tinyData_.capacity();
++stats.numShared;
}
if (entry->isPrefetch_) {
++stats.numPrefetch;
stats.prefetchBytes += entry->size();
}
++stats.numEntries;
stats.tinySize += entry->tinyData_.size();
stats.tinyPadding += entry->tinyData_.capacity() - entry->tinyData_.size();
Expand Down
8 changes: 7 additions & 1 deletion velox/common/caching/AsyncDataCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -787,9 +787,15 @@ class AsyncDataCache : public memory::Cache {
/// Returns true if there is an entry for 'key'. Updates access time.
bool exists(RawFileCacheKey key) const;

#if defined(__has_feature)
#if __has_feature(thread_sanitizer)
__attribute__((__no_sanitize__("thread")))
#endif
#endif
/// Returns snapshot of the aggregated stats from all shards and the stats of
/// SSD cache if used.
virtual CacheStats refreshStats() const;
virtual CacheStats
refreshStats() const;

/// If 'details' is true, returns the stats of the backing memory allocator
/// and ssd cache. Otherwise, only returns the cache stats.
Expand Down

0 comments on commit cf2ba83

Please sign in to comment.