Handle failures in block-based table size/offset approximation

[ajkr]

In crash test with fault injection, we were seeing stack traces like the following:

#3 0x00007f75f763c533 in __GI___assert_fail (assertion=assertion@entry=0x1c5b2a0 "end_offset >= start_offset", file=file@entry=0x1c580a0 "table/block_based/block_based_table_reader.cc", line=line@entry=3245,
function=function@entry=0x1c60e60 "virtual uint64_t rocksdb::BlockBasedTable::ApproximateSize(const rocksdb::Slice&, const rocksdb::Slice&, rocksdb::TableReaderCaller)") at assert.c:101
#4 0x00000000010ea9b4 in rocksdb::BlockBasedTable::ApproximateSize (this=<optimized out>, start=..., end=..., caller=<optimized out>) at table/block_based/block_based_table_reader.cc:3224
#5 0x0000000000be61fb in rocksdb::TableCache::ApproximateSize (this=0x60f0000161b0, start=..., end=..., fd=..., caller=caller@entry=rocksdb::kCompaction, internal_comparator=..., prefix_extractor=...) at db/table_cache.cc:719
#6 0x0000000000c3eaec in rocksdb::VersionSet::ApproximateSize (this=<optimized out>, v=<optimized out>, f=..., start=..., end=..., caller=<optimized out>) at ./db/version_set.h:850
#7 0x0000000000c6ebc3 in rocksdb::VersionSet::ApproximateSize (this=<optimized out>, options=..., v=v@entry=0x621000047500, start=..., end=..., start_level=start_level@entry=0, end_level=<optimized out>, caller=<optimized out>)
at db/version_set.cc:5657
#8 0x000000000166e894 in rocksdb::CompactionJob::GenSubcompactionBoundaries (this=<optimized out>) at ./include/rocksdb/options.h:1869
#9 0x000000000168c526 in rocksdb::CompactionJob::Prepare (this=this@entry=0x7f75f3ffcf00) at db/compaction/compaction_job.cc:546

The problem occurred in ApproximateSize() when the index Seek() for the first ApproximateDataOffsetOf() encountered an I/O error, while the second Seek() did not. In the old code that scenario caused start_offset == data_size , thus it was easy to trip the assertion that end_offset >= start_offset.

The fix is to set start_offset == 0 when the first index Seek() fails, and end_offset == data_size when the second index Seek() fails. I doubt these give an "on average correct" answer for how this function is used, but I/O errors in index seeks are hopefully rare, it looked consistent with what was already there, and it was easier to calculate.

Test Plan: run the repro command for a while and stopped seeing coredumps -

$ while !  ./db_stress --block_size=128 --cache_size=32768 --clear_column_family_one_in=0 --column_families=1 --continuous_verification_interval=0 --db=/dev/shm/rocksdb_crashtest --delpercent=4 --delrangepercent=1 --destroy_db_initially=0 --expected_values_dir=/dev/shm/rocksdb_crashtest_expected --index_type=2 --iterpercent=10  --kill_random_test=18887 --max_key=1000000 --max_bytes_for_level_base=2048576 --nooverwritepercent=1 --open_files=-1 --open_read_fault_one_in=32 --ops_per_thread=1000000 --prefixpercent=5 --read_fault_one_in=0 --readpercent=45 --reopen=0 --skip_verifydb=1 --subcompactions=2 --target_file_size_base=524288 --test_batches_snapshots=0 --value_size_mult=32 --write_buffer_size=524288 --writepercent=35  ; do : ; done

[facebook-github-bot]

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

[pdillinger]

LGTM

[siying]

Wow, good catch. How did we discover it?

[ajkr]

LGTM

Thanks for the review!

Wow, good catch. How did we discover it?

Thanks. We got a couple crash test failure tasks already for Assertion 'end_offset >= start_offset' failed. I had remembered that fault injection was enabled in both cases. Mostly through code inspection I found the bug.

It was nearly impossible to repro before understanding the bug because it can only happen in a very specific setup. Fault injection for {read,write}_fault_one_in are specific to the thread executing the operation so never inject errors to background threads. However, GetApproximateSize() looks to typically be called in the background for subcompaction partitioning.

It turns out there is one case when we do inject faults in the background. That is, when open_metadata_write_fault_one_in > 0, there is a very short time window after the DB opens while fault injection is enabled in all threads. During that short window, it's possible we inject fault into a background thread that's picking subcompaction partitions. Knowing that, it's possible to design a crash test repro command that has subcompaction enabled, open_metadata_write_fault_one_in > 0, and crashes/reopens at a high frequency.