Skip to content

Commit

Permalink
Fix data races caught by tsan
Browse files Browse the repository at this point in the history
Summary:
This fixes the tsan build failures in:
- write_callback_test
- persistent_cache_test.*
Closes facebook#2339

Differential Revision: D5101190

Pulled By: sagar0

fbshipit-source-id: 537e19ed05272b1f34cfbf793aa822b2264a1643
  • Loading branch information
sagar0 authored and facebook-github-bot committed May 22, 2017
1 parent 4c9d2b1 commit 228f49d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 12 deletions.
17 changes: 13 additions & 4 deletions db/write_callback_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#ifndef ROCKSDB_LITE

#include <atomic>
#include <functional>
#include <string>
#include <utility>
Expand Down Expand Up @@ -65,11 +66,19 @@ class WriteCallbackTestWriteCallback2 : public WriteCallback {
class MockWriteCallback : public WriteCallback {
public:
bool should_fail_ = false;
bool was_called_ = false;
bool allow_batching_ = false;
std::atomic<bool> was_called_{false};

MockWriteCallback() {}

MockWriteCallback(const MockWriteCallback& other) {
should_fail_ = other.should_fail_;
allow_batching_ = other.allow_batching_;
was_called_.store(other.was_called_.load());
}

Status Callback(DB* db) override {
was_called_ = true;
was_called_.store(true);
if (should_fail_) {
return Status::Busy();
} else {
Expand All @@ -92,7 +101,7 @@ TEST_F(WriteCallbackTest, WriteWithCallbackTest) {
void Clear() {
kvs_.clear();
write_batch_.Clear();
callback_.was_called_ = false;
callback_.was_called_.store(false);
}

MockWriteCallback callback_;
Expand Down Expand Up @@ -265,7 +274,7 @@ TEST_F(WriteCallbackTest, WriteWithCallbackTest) {
// check for keys
string value;
for (auto& w : write_group) {
ASSERT_TRUE(w.callback_.was_called_);
ASSERT_TRUE(w.callback_.was_called_.load());
for (auto& kvp : w.kvs_) {
if (w.callback_.should_fail_) {
ASSERT_TRUE(
Expand Down
9 changes: 5 additions & 4 deletions utilities/persistent_cache/block_cache_tier.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <unistd.h>
#endif // ! OS_WIN

#include <atomic>
#include <list>
#include <memory>
#include <set>
Expand Down Expand Up @@ -123,10 +124,10 @@ class BlockCacheTier : public PersistentCacheTier {
HistogramImpl read_hit_latency_;
HistogramImpl read_miss_latency_;
HistogramImpl write_latency_;
uint64_t cache_hits_ = 0;
uint64_t cache_misses_ = 0;
uint64_t cache_errors_ = 0;
uint64_t insert_dropped_ = 0;
std::atomic<uint64_t> cache_hits_{0};
std::atomic<uint64_t> cache_misses_{0};
std::atomic<uint64_t> cache_errors_{0};
std::atomic<uint64_t> insert_dropped_{0};

double CacheHitPct() const {
const auto lookups = cache_hits_ + cache_misses_;
Expand Down
8 changes: 4 additions & 4 deletions utilities/persistent_cache/volatile_tier_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ class VolatileCacheTier : public PersistentCacheTier {
};

struct Statistics {
uint64_t cache_misses_ = 0;
uint64_t cache_hits_ = 0;
uint64_t cache_inserts_ = 0;
uint64_t cache_evicts_ = 0;
std::atomic<uint64_t> cache_misses_{0};
std::atomic<uint64_t> cache_hits_{0};
std::atomic<uint64_t> cache_inserts_{0};
std::atomic<uint64_t> cache_evicts_{0};

double CacheHitPct() const {
auto lookups = cache_hits_ + cache_misses_;
Expand Down

0 comments on commit 228f49d

Please sign in to comment.