Cache fragmented range tombstone list for mutable memtables #10547
Conversation
cbi42
changed the title
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
|
cbi42
force-pushed
the
cache-tombstone-read-path
branch
from
3489062
to
41cccc8
Compare
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
cbi42
force-pushed
the
cache-tombstone-read-path
branch
from
41cccc8
to
6bbe600
Compare
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
cbi42
force-pushed
the
cache-tombstone-read-path
branch
from
6bbe600
to
d8d5fc4
Compare
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
cbi42
changed the title
cbi42
force-pushed
the
cache-tombstone-read-path
branch
from
d8d5fc4
to
7740896
Compare
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: Optionally issue DeleteRange in `*whilewriting` benchmarks. This happens in `BGWriter` and uses similar logic as in `DoWrite` to issue DeleteRange operations. I added this when I was benchmarking #10547, but this should be an independent PR. Pull Request resolved: #10552 Test Plan: ran some benchmarks with various delete range options, e.g. `./db_bench --benchmarks=readwhilewriting --writes_per_range_tombstone=100 --writes=200000 --reads=1000000 --disable_auto_compactions --max_num_range_tombstones=10000` Reviewed By: ajkr Differential Revision: D38927020 Pulled By: cbi42 fbshipit-source-id: 31ee20cb8127f7173f0816ea0cc2a204ec02aad6
cbi42
force-pushed
the
cache-tombstone-read-path
branch
from
7740896
to
9968f98
Compare
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
cbi42
force-pushed
the
cache-tombstone-read-path
branch
from
9968f98
to
1907503
Compare
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
cbi42
force-pushed
the
cache-tombstone-read-path
branch
from
d4ca7e8
to
af3b15e
Compare
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: fix a data race introduced in #10547 (P5295241720), first reported by pdillinger. The race is between the `std::atomic_load_explicit` in NewRangeTombstoneIteratorInternal and the `std::atomic_store_explicit` in MemTable::Add() that operate on `cached_range_tombstone_`. P5295241720 shows that `atomic_store_explicit` initializes some mutex which `atomic_load_explicit` could be trying to call `lock()` on at the same time. This fix moves the initialization to memtable constructor. Pull Request resolved: #10680 Test Plan: `USE_CLANG=1 COMPILE_WITH_TSAN=1 make -j24 whitebox_crash_test` Reviewed By: ajkr Differential Revision: D39528696 Pulled By: cbi42 fbshipit-source-id: ee740841044438e18ad2b8ea567444dd542dd8e2
…acebook#10547)" This reverts commit f291eef.
Summary: Each read from memtable used to read and fragment all the range tombstones into a
FragmentedRangeTombstoneList. #10380 improved the inefficient here by caching aFragmentedRangeTombstoneListwith each immutable memtable. This PR extends the caching to mutable memtables. The fragmented range tombstone can be constructed in either read (This PR) or write path (#10584). With both implementation, eachDeleteRange()will invalidate the cache, and the difference is where the cache is re-constructed.CoreLocalArrayis used to store the cache with each memtable so that multi-threaded reads can be efficient. More specifically, each core will have a shared_ptr to a shared_ptr pointing to the current cache. Each read thread will only update the reference count in its core-local shared_ptr, and this is only needed when reading from mutable memtables.The choice between write path and read path is not an easy one: they are both improvement compared to no caching in the current implementation, but they favor different operations and could cause regression in the other operation (read vs write). The write path caching in (#10584) leads to a cleaner implementation, but I chose the read path caching here to avoid significant regression in write performance when there is a considerable amount of range tombstones in a single memtable (the number from the benchmark below suggests >1000 with concurrent writers). Note that even though the fragmented range tombstone list is only constructed in
DeleteRange()operations, it could block other writes from proceeding, and hence affects overall write performance.Test plan:
TestGet()to verify against expected state #10553 to compare Get() result against expected state:./db_stress_branch --readpercent=57 --prefixpercent=4 --writepercent=25 -delpercent=5 --iterpercent=5 --delrangepercent=4Write perf regressed since the cost of constructing fragmented range tombstone list is shifted from every read to a single write. 6cbe5d8 is included in the last column as a reference to see performance impact on multi-thread reads if
CoreLocalArrayis not used.micros/op averaged over 5 runs: first 4 columns are for fillrandom, last 4 columns are for readrandom.
readwhilewritingfrom Optionally issueDeleteRangein*whilewritingbenchmarks #10552, 100 range tombstones are written:./db_bench --benchmarks=readwhilewriting --writes_per_range_tombstone=500 --max_write_buffer_number=100 --min_write_buffer_number_to_merge=100 --writes=100000 --reads=500000 --disable_auto_compactions --max_num_range_tombstones=10000 --finish_after_writesreadrandom micros/op: