Fix some errors in async prefetching in FilePrefetchBuffer #9734

akankshamahajan15 · 2022-03-22T20:10:38Z

Summary: In ReadOption async_io which prefetches the data asynchronously, db_bench and db_stress runs were failing because wrong data was prefetched which resulted in Error: Checksum mismatched. Wrong data was copied because capacity was less than actual size needed. It has been fixed in this PR.

Since there are two separate methods for async and sync prefetching, these changes are in async prefetching methods and any changes would not effect normal prefetching. I ran the regressions to make sure normal prefetching is fine.

Test Plan:

CircleCI jobs
Ran db_bench

. /db_bench -use_existing_db=true
-db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32
-value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680
-duration=120 -ops_between_duration_checks=1 -async_io=1 -adaptive_readahead=1

Ran db_stress test

export CRASH_TEST_EXT_ARGS=" --async_io=1 --adaptive_readahead=1"
make crash_test -j

Run regressions for async_io disabled.

Old flow without any async changes:

./db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1
Initializing RocksDB Options from the specified file
Initializing RocksDB Options from command-line flags
RocksDB:    version 7.0
Date:       Thu Mar 17 13:11:34 2022
CPU:        24 * Intel Core Processor (Broadwell)
CPUCache:   16384 KB
Keys:       32 bytes each (+ 0 bytes user-defined timestamp)
Values:     512 bytes each (256 bytes after compression)
Entries:    5000000
Prefix:    0 bytes
Keys per prefix:    0
RawSize:    2594.0 MB (estimated)
FileSize:   1373.3 MB (estimated)
Write rate: 0 bytes/second
Read rate: 0 ops/second
Compression: Snappy
Compression sampling rate: 0
Memtablerep: SkipListFactory
Perf Level: 1
------------------------------------------------
DB path: [/tmp/prefix_scan_prefetch_main]
seekrandom   :  483618.390 micros/op 2 ops/sec;  338.9 MB/s (249 of 249 found)

With async prefetching changes and async_io disabled to make sure in normal prefetching there is no regression.

./db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1 --async_io=0
Initializing RocksDB Options from the specified file
Initializing RocksDB Options from command-line flags
RocksDB:    version 7.1
Date:       Wed Mar 23 15:56:37 2022
CPU:        24 * Intel Core Processor (Broadwell)
CPUCache:   16384 KB
Keys:       32 bytes each (+ 0 bytes user-defined timestamp)
Values:     512 bytes each (256 bytes after compression)
Entries:    5000000
Prefix:    0 bytes
Keys per prefix:    0
RawSize:    2594.0 MB (estimated)
FileSize:   1373.3 MB (estimated)
Write rate: 0 bytes/second
Read rate: 0 ops/second
Compression: Snappy
Compression sampling rate: 0
Memtablerep: SkipListFactory
Perf Level: 1
------------------------------------------------
DB path: [/tmp/prefix_scan_prefetch_main]
seekrandom   :  481819.816 micros/op 2 ops/sec;  340.2 MB/s (250 of 250 found)

facebook-github-bot · 2022-03-22T20:44:05Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2022-03-23T22:05:59Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

Summary: db_bench was failing due to some corner cases which has been fixed in this PR. Test Plan: ./db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1 -async_io=1

facebook-github-bot · 2022-03-23T22:17:25Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-03-23T22:26:23Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

riversand963 · 2022-03-23T23:06:45Z

It's helpful to include the original error message in the PR description.

facebook-github-bot · 2022-03-24T14:49:05Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

akankshamahajan15 · 2022-03-25T21:33:21Z

file/file_prefetch_buffer.cc

@@ -66,6 +66,17 @@ void FilePrefetchBuffer::CalculateOffsetAndLen(size_t alignment,
    // chunk_len is greater than 0.
    bufs_[index].buffer_.RefitTail(static_cast<size_t>(chunk_offset_in_buffer),
                                   static_cast<size_t>(chunk_len));
+  } else if (chunk_len > 0) {


For async prefetching we don’t refit the data so it allocate new memory. Planning to handle this in next iteration

I didn't understand this case. By the time we get here, PrefetchAsync would have already consumed as much data as it can from the buffers. IIUC, one or both buffers should be empty, i.e chunk_len will be 0. When exactly will this case be true?

This happens when data is in cache and it updates the read pattern in FilePrefetchBuffer because of which when it reads the next pattern, it needs to prefetch the data and some chunk is left in the buffer.

One example:

1. Read offset: 5649638, size: 4774 2. curr: 1, bufs_[curr_].offset_: 5623808, bufs_[curr_].buffer_.CurrentSize(): 16384, bufs_[curr_ ^1].offset: 5640192, bufs_[curr_^1].buffer_.CurrentSize(): 12288 3. For offset: 5649638, prev_offset: 5644838, prev_len: 4800 (For previous offset: In cache offset: 5644838). 4. So it clears the first buffer, swap it with second and second buffer has chunk_len : 4096.

I will try to add the unit test to catch this error. But this was faced in db_stress

riversand963

Only verified that this PR affects only an experimental feature readOptions::async_io. Will defer to @anand1976 to fully evaluate.

facebook-github-bot · 2022-03-25T22:43:46Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-03-25T22:47:59Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: In ReadOption `async_io` which prefetches the data asynchronously, db_bench and db_stress runs were failing because wrong data was prefetched which resulted in Error: Checksum mismatched. Wrong data was copied because capacity was less than actual size needed. It has been fixed in this PR. Since there are two separate methods for async and sync prefetching, these changes are in async prefetching methods and any changes would not effect normal prefetching. I ran the regressions to make sure normal prefetching is fine. Pull Request resolved: #9734 Test Plan: 1. CircleCI jobs 2. Ran db_bench ``` . /db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1 -async_io=1 -adaptive_readahead=1 ``` 3. Ran db_stress test ``` export CRASH_TEST_EXT_ARGS=" --async_io=1 --adaptive_readahead=1" make crash_test -j ``` 4. Run regressions for async_io disabled. Old flow without any async changes: ``` ./db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1 Initializing RocksDB Options from the specified file Initializing RocksDB Options from command-line flags RocksDB: version 7.0 Date: Thu Mar 17 13:11:34 2022 CPU: 24 * Intel Core Processor (Broadwell) CPUCache: 16384 KB Keys: 32 bytes each (+ 0 bytes user-defined timestamp) Values: 512 bytes each (256 bytes after compression) Entries: 5000000 Prefix: 0 bytes Keys per prefix: 0 RawSize: 2594.0 MB (estimated) FileSize: 1373.3 MB (estimated) Write rate: 0 bytes/second Read rate: 0 ops/second Compression: Snappy Compression sampling rate: 0 Memtablerep: SkipListFactory Perf Level: 1 ------------------------------------------------ DB path: [/tmp/prefix_scan_prefetch_main] seekrandom : 483618.390 micros/op 2 ops/sec; 338.9 MB/s (249 of 249 found) ``` With async prefetching changes and async_io disabled to make sure in normal prefetching there is no regression. ``` ./db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1 --async_io=0 Initializing RocksDB Options from the specified file Initializing RocksDB Options from command-line flags RocksDB: version 7.1 Date: Wed Mar 23 15:56:37 2022 CPU: 24 * Intel Core Processor (Broadwell) CPUCache: 16384 KB Keys: 32 bytes each (+ 0 bytes user-defined timestamp) Values: 512 bytes each (256 bytes after compression) Entries: 5000000 Prefix: 0 bytes Keys per prefix: 0 RawSize: 2594.0 MB (estimated) FileSize: 1373.3 MB (estimated) Write rate: 0 bytes/second Read rate: 0 ops/second Compression: Snappy Compression sampling rate: 0 Memtablerep: SkipListFactory Perf Level: 1 ------------------------------------------------ DB path: [/tmp/prefix_scan_prefetch_main] seekrandom : 481819.816 micros/op 2 ops/sec; 340.2 MB/s (250 of 250 found) ``` Reviewed By: riversand963 Differential Revision: D35058471 Pulled By: akankshamahajan15 fbshipit-source-id: 9233a1e6d97cea0c7a8111bfb9e8ac3251c341ce

…9734) Summary: In ReadOption `async_io` which prefetches the data asynchronously, db_bench and db_stress runs were failing because wrong data was prefetched which resulted in Error: Checksum mismatched. Wrong data was copied because capacity was less than actual size needed. It has been fixed in this PR. Since there are two separate methods for async and sync prefetching, these changes are in async prefetching methods and any changes would not effect normal prefetching. I ran the regressions to make sure normal prefetching is fine. Pull Request resolved: facebook#9734 Test Plan: 1. CircleCI jobs 2. Ran db_bench ``` . /db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1 -async_io=1 -adaptive_readahead=1 ``` 3. Ran db_stress test ``` export CRASH_TEST_EXT_ARGS=" --async_io=1 --adaptive_readahead=1" make crash_test -j ``` 4. Run regressions for async_io disabled. Old flow without any async changes: ``` ./db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1 Initializing RocksDB Options from the specified file Initializing RocksDB Options from command-line flags RocksDB: version 7.0 Date: Thu Mar 17 13:11:34 2022 CPU: 24 * Intel Core Processor (Broadwell) CPUCache: 16384 KB Keys: 32 bytes each (+ 0 bytes user-defined timestamp) Values: 512 bytes each (256 bytes after compression) Entries: 5000000 Prefix: 0 bytes Keys per prefix: 0 RawSize: 2594.0 MB (estimated) FileSize: 1373.3 MB (estimated) Write rate: 0 bytes/second Read rate: 0 ops/second Compression: Snappy Compression sampling rate: 0 Memtablerep: SkipListFactory Perf Level: 1 ------------------------------------------------ DB path: [/tmp/prefix_scan_prefetch_main] seekrandom : 483618.390 micros/op 2 ops/sec; 338.9 MB/s (249 of 249 found) ``` With async prefetching changes and async_io disabled to make sure in normal prefetching there is no regression. ``` ./db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1 --async_io=0 Initializing RocksDB Options from the specified file Initializing RocksDB Options from command-line flags RocksDB: version 7.1 Date: Wed Mar 23 15:56:37 2022 CPU: 24 * Intel Core Processor (Broadwell) CPUCache: 16384 KB Keys: 32 bytes each (+ 0 bytes user-defined timestamp) Values: 512 bytes each (256 bytes after compression) Entries: 5000000 Prefix: 0 bytes Keys per prefix: 0 RawSize: 2594.0 MB (estimated) FileSize: 1373.3 MB (estimated) Write rate: 0 bytes/second Read rate: 0 ops/second Compression: Snappy Compression sampling rate: 0 Memtablerep: SkipListFactory Perf Level: 1 ------------------------------------------------ DB path: [/tmp/prefix_scan_prefetch_main] seekrandom : 481819.816 micros/op 2 ops/sec; 340.2 MB/s (250 of 250 found) ``` Reviewed By: riversand963 Differential Revision: D35058471 Pulled By: akankshamahajan15 fbshipit-source-id: 9233a1e6d97cea0c7a8111bfb9e8ac3251c341ce

facebook-github-bot added the CLA Signed label Mar 22, 2022

akankshamahajan15 force-pushed the async_io branch from 90fade1 to ae1a6e2 Compare March 22, 2022 20:11

akankshamahajan15 requested a review from riversand963 March 22, 2022 20:43

akankshamahajan15 force-pushed the async_io branch from ae1a6e2 to 11b4d41 Compare March 23, 2022 22:05

akankshamahajan15 changed the title ~~Fix some failures in async_io readoption in FilePrefetchBuffer~~ Fix some errors in async prefetching in FilePrefetchBuffer Mar 23, 2022

akankshamahajan15 force-pushed the async_io branch from 11b4d41 to 10880b8 Compare March 23, 2022 22:17

riversand963 changed the base branch from main to 7.0.fb March 23, 2022 23:16

riversand963 changed the base branch from 7.0.fb to main March 23, 2022 23:17

akankshamahajan15 requested a review from anand1976 March 23, 2022 23:23

akankshamahajan15 commented Mar 25, 2022

View reviewed changes

riversand963 approved these changes Mar 25, 2022

View reviewed changes

Merge branch 'main' into async_io

ee89d7b

facebook-github-bot closed this in 33f8a08 Mar 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix some errors in async prefetching in FilePrefetchBuffer #9734

Fix some errors in async prefetching in FilePrefetchBuffer #9734

akankshamahajan15 commented Mar 22, 2022 •

edited

Loading

facebook-github-bot commented Mar 22, 2022

facebook-github-bot commented Mar 23, 2022

facebook-github-bot commented Mar 23, 2022

facebook-github-bot commented Mar 23, 2022

riversand963 commented Mar 23, 2022

facebook-github-bot commented Mar 24, 2022

akankshamahajan15 Mar 25, 2022

anand1976 Apr 1, 2022

akankshamahajan15 Apr 4, 2022

akankshamahajan15 Apr 4, 2022

riversand963 left a comment

facebook-github-bot commented Mar 25, 2022

facebook-github-bot commented Mar 25, 2022

Fix some errors in async prefetching in FilePrefetchBuffer #9734

Fix some errors in async prefetching in FilePrefetchBuffer #9734

Conversation

akankshamahajan15 commented Mar 22, 2022 • edited Loading

facebook-github-bot commented Mar 22, 2022

facebook-github-bot commented Mar 23, 2022

facebook-github-bot commented Mar 23, 2022

facebook-github-bot commented Mar 23, 2022

riversand963 commented Mar 23, 2022

facebook-github-bot commented Mar 24, 2022

akankshamahajan15 Mar 25, 2022

Choose a reason for hiding this comment

anand1976 Apr 1, 2022

Choose a reason for hiding this comment

akankshamahajan15 Apr 4, 2022

Choose a reason for hiding this comment

akankshamahajan15 Apr 4, 2022

Choose a reason for hiding this comment

riversand963 left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Mar 25, 2022

facebook-github-bot commented Mar 25, 2022

akankshamahajan15 commented Mar 22, 2022 •

edited

Loading