Fix data race reported on SetIsInSecondaryCache in LRUCache (#10472)

Summary:
Currently, `SetIsInSecondaryCache` is after `Promote`. After `Promote`, a handle can be accessed and its flags can be set. This causes data race.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10472

Test Plan:
unit tests
stress tests

Reviewed By: pdillinger

Differential Revision: D38403991

Pulled By: gitbw95

fbshipit-source-id: 0aaa2d2edeaf5bc799fcce605648fe49eb7119c2
main
Bo Wang 2 years ago committed by Facebook GitHub Bot
parent bf4532eb5c
commit f28d0c2020
  1. 1
      HISTORY.md
  2. 3
      cache/lru_cache.cc

@ -20,6 +20,7 @@
* Fix data race bug in hash linked list memtable. With this bug, read request might temporarily miss an old record in the memtable in a race condition to the hash bucket. * Fix data race bug in hash linked list memtable. With this bug, read request might temporarily miss an old record in the memtable in a race condition to the hash bucket.
* Fix a bug that `best_efforts_recovery` may fail to open the db with mmap read. * Fix a bug that `best_efforts_recovery` may fail to open the db with mmap read.
* Fixed a bug where blobs read during compaction would pollute the cache. * Fixed a bug where blobs read during compaction would pollute the cache.
* Fixed a data race in LRUCache when used with a secondary_cache.
### Behavior Change ### Behavior Change
* Added checksum handshake during the copying of decompressed WAL fragment. This together with #9875, #10037, #10212, #10114 and #10319 provides end-to-end integrity protection for write batch during recovery. * Added checksum handshake during the copying of decompressed WAL fragment. This together with #9875, #10037, #10212, #10114 and #10319 provides end-to-end integrity protection for write batch during recovery.

@ -460,10 +460,10 @@ Cache::Handle* LRUCacheShard::Lookup(
e->sec_handle = secondary_handle.release(); e->sec_handle = secondary_handle.release();
e->total_charge = 0; e->total_charge = 0;
e->Ref(); e->Ref();
e->SetIsInSecondaryCache(is_in_sec_cache);
if (wait) { if (wait) {
Promote(e); Promote(e);
e->SetIsInSecondaryCache(is_in_sec_cache);
if (!e->value) { if (!e->value) {
// The secondary cache returned a handle, but the lookup failed. // The secondary cache returned a handle, but the lookup failed.
e->Unref(); e->Unref();
@ -477,7 +477,6 @@ Cache::Handle* LRUCacheShard::Lookup(
// If wait is false, we always return a handle and let the caller // If wait is false, we always return a handle and let the caller
// release the handle after checking for success or failure. // release the handle after checking for success or failure.
e->SetIncomplete(true); e->SetIncomplete(true);
e->SetIsInSecondaryCache(is_in_sec_cache);
// This may be slightly inaccurate, if the lookup eventually fails. // This may be slightly inaccurate, if the lookup eventually fails.
// But the probability is very low. // But the probability is very low.
PERF_COUNTER_ADD(secondary_cache_hit_count, 1); PERF_COUNTER_ADD(secondary_cache_hit_count, 1);

Loading…
Cancel
Save