Commit Graph

10640 Commits

Author SHA1 Message Date
Levi Tamasi
7cc52cd8f5 Update HISTORY for PR 8994 (#9017)
Summary:
Also, expand on/clarify a comment in `VersionStorageInfoTest`.

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

Reviewed By: riversand963

Differential Revision: D31566130

Pulled By: ltamasi

fbshipit-source-id: 1d30c7af084c4de7b2030bc6c768838d65746010
2021-10-12 10:19:56 -07:00
Giuseppe Ottaviano
22d4dc5066 Fix race in WriteBufferManager (#9009)
Summary:
EndWriteStall has a data race: `queue_.empty()` is checked outside of the
mutex, so once we enter the critical section another thread may already have
cleared the list, and accessing the `front()` is undefined behavior (and causes
interesting crashes under high concurrency).

This PR fixes the bug, and also rewrites the logic to make it easier to reason
about it. It also fixes another subtle bug: if some writers are stalled and
`SetBufferSize(0)` is called, which disables the WBM, the writer are not
unblocked because of an early `enabled()` check in `EndWriteStall()`.

It doesn't significantly change the locking behavior, as before writers won't
lock unless entering a stall condition, and `FreeMem` almost always locks if
stalling is allowed, but that is inevitable with the current design. Liveness is
guaranteed by the fact that if some writes are blocked, eventually all writes
will be blocked due to `stall_active_`, and eventually all memory is freed.

While at it, do a couple of optimizations:

- In `WBMStallInterface::Signal()` signal the CV only after releasing the
  lock. Signaling under the lock is a common pitfall, as it causes the woken-up
  thread to immediately go back to sleep because the mutex is still locked by
  the awaker.

- Move all allocations and deallocations outside of the lock.

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

Test Plan:
```
USE_CLANG=1 make -j64 all check
```

Reviewed By: akankshamahajan15

Differential Revision: D31550668

Pulled By: ot

fbshipit-source-id: 5125387c3dc7ecaaa2b8bbc736e58c4156698580
2021-10-12 00:16:21 -07:00
Yanqin Jin
e1139167ae Inline an empty destructor (#9004)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9004

Inline an empty destructor

Reviewed By: ltamasi

Differential Revision: D31525561

fbshipit-source-id: 3b9e37f06b0c70529a5d2d660de21ea335c73611
2021-10-11 18:14:10 -07:00
Yanqin Jin
1a79839c59 Some code cleanup (#9003)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9003

cleanup some code before real work.

Reviewed By: ltamasi

Differential Revision: D31525563

fbshipit-source-id: 44558b3594f2200adc7d8621b08b06c77e358a27
2021-10-11 18:14:10 -07:00
Levi Tamasi
3e1bf771a3 Make it possible to force the garbage collection of the oldest blob files (#8994)
Summary:
The current BlobDB garbage collection logic works by relocating the valid
blobs from the oldest blob files as they are encountered during compaction,
and cleaning up blob files once they contain nothing but garbage. However,
with sufficiently skewed workloads, it is theoretically possible to end up in a
situation when few or no compactions get scheduled for the SST files that contain
references to the oldest blob files, which can lead to increased space amp due
to the lack of GC.

In order to efficiently handle such workloads, the patch adds a new BlobDB
configuration option called `blob_garbage_collection_force_threshold`,
which signals to BlobDB to schedule targeted compactions for the SST files
that keep alive the oldest batch of blob files if the overall ratio of garbage in
the given blob files meets the threshold *and* all the given blob files are
eligible for GC based on `blob_garbage_collection_age_cutoff`. (For example,
if the new option is set to 0.9, targeted compactions will get scheduled if the
sum of garbage bytes meets or exceeds 90% of the sum of total bytes in the
oldest blob files, assuming all affected blob files are below the age-based cutoff.)
The net result of these targeted compactions is that the valid blobs in the oldest
blob files are relocated and the oldest blob files themselves cleaned up (since
*all* SST files that rely on them get compacted away).

These targeted compactions are similar to periodic compactions in the sense
that they force certain SST files that otherwise would not get picked up to undergo
compaction and also in the sense that instead of merging files from multiple levels,
they target a single file. (Note: such compactions might still include neighboring files
from the same level due to the need of having a "clean cut" boundary but they never
include any files from any other level.)

This functionality is currently only supported with the leveled compaction style
and is inactive by default (since the default value is set to 1.0, i.e. 100%).

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

Test Plan: Ran `make check` and tested using `db_bench` and the stress/crash tests.

Reviewed By: riversand963

Differential Revision: D31489850

Pulled By: ltamasi

fbshipit-source-id: 44057d511726a0e2a03c5d9313d7511b3f0c4eab
2021-10-11 18:03:01 -07:00
Andrew Kryczka
a282eff3d1 Protect existing files in FaultInjectionTest{Env,FS}::ReopenWritableFile() (#8995)
Summary:
`FaultInjectionTest{Env,FS}::ReopenWritableFile()` functions were accidentally deleting WALs from previous `db_stress` runs causing verification to fail. They were operating under the assumption that `ReopenWritableFile()` would delete any existing file. It was a reasonable assumption considering the `{Env,FileSystem}::ReopenWritableFile()` documentation stated that would happen. The only problem was neither the implementations we offer nor the "real" clients in RocksDB code followed that contract. So, this PR updates the contract as well as fixing the fault injection client usage.

The fault injection change exposed that `ExternalSSTFileBasicTest.SyncFailure` was relying on a fault injection `Env` dropping unsynced data written by a regular `Env`. I changed that test to make its `SstFileWriter` use fault injection `Env`, and also implemented `LinkFile()` in fault injection so the unsynced data is tracked under the new name.

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

Test Plan:
- Verified it fixes the following failure:

```
$ ./db_stress --clear_column_family_one_in=0 --column_families=1 --db=/dev/shm/rocksdb_crashtest_whitebox --delpercent=5 --expected_values_dir=/dev/shm/rocksdb_crashtest_expected --iterpercent=0 --key_len_percent_dist=1,30,69 --max_key=100000 --max_key_len=3 --nooverwritepercent=1 --ops_per_thread=1000 --prefixpercent=0 --readpercent=60 --reopen=0 --target_file_size_base=1048576 --test_batches_snapshots=0 --write_buffer_size=1048576 --writepercent=35 --value_size_mult=33 -threads=1
...
$ ./db_stress --avoid_flush_during_recovery=1 --clear_column_family_one_in=0 --column_families=1 --db=/dev/shm/rocksdb_crashtest_whitebox --delpercent=5 --destroy_db_initially=0 --expected_values_dir=/dev/shm/rocksdb_crashtest_expected --iterpercent=10 --key_len_percent_dist=1,30,69 --max_bytes_for_level_base=4194304 --max_key=100000 --max_key_len=3 --nooverwritepercent=1 --open_files=-1 --open_metadata_write_fault_one_in=8 --open_write_fault_one_in=16 --ops_per_thread=1000 --prefix_size=-1 --prefixpercent=0 --readpercent=50 --sync=1 --target_file_size_base=1048576 --test_batches_snapshots=0 --write_buffer_size=1048576 --writepercent=35 --value_size_mult=33 -threads=1
...
Verification failed for column family 0 key 000000000000001300000000000000857878787878 (1143): Value not found: NotFound:
Crash-recovery verification failed :(
...
```

- `make check -j48`

Reviewed By: ltamasi

Differential Revision: D31495388

Pulled By: ajkr

fbshipit-source-id: 7886ccb6a07cb8b78ad7b6c1c341ccf40bb68385
2021-10-11 16:23:18 -07:00
Andrew Kryczka
ee239df351 Initialize cache dumper DumpUnit in constructor (#9014)
Summary:
Should fix clang-analyze:

```
utilities/cache_dump_load_impl.cc:296:38: warning: The left operand of '!=' is a garbage value
  while (io_s.ok() && dump_unit.type != CacheDumpUnitType::kFooter) {
                      ~~~~~~~~~~~~~~ ^
```

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

Reviewed By: zhichao-cao

Differential Revision: D31546912

Pulled By: ajkr

fbshipit-source-id: a2e0dc7874e8c1c6abf190862b5d49e6a6ad6d01
2021-10-11 13:05:35 -07:00
Hui Xiao
6c3bf83d6f Update HISTORY.md for #8428 (#9001)
Summary:
Context:
HISTORY.md was not properly updated along with the change in https://github.com/facebook/rocksdb/pull/8428, where we introduced a change of accounting compression dictionary buffering memory and an extra condition of triggering data unbuffering.
Updated HISTORY.md for https://github.com/facebook/rocksdb/pull/8428 in 6.25.0 HISTORY.md section.
Updated blog post https://rocksdb.org/blog/2021/05/31/dictionary-compression.html.

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

Reviewed By: ajkr

Differential Revision: D31517836

Pulled By: hx235

fbshipit-source-id: 01f6b30de4e1ff6b315aa8221139d9b700c7c629
2021-10-08 17:00:30 -07:00
anand76
418831cf45 Fix wrong tmp dir name in fbcode stress test lego command (#9000)
Summary:
Change the directory to /dev/shm/rocksdb_fbcode_crash_test.

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

Reviewed By: akankshamahajan15

Differential Revision: D31511886

Pulled By: anand1976

fbshipit-source-id: 4e48fd2338638fd9223b55d5f7496c26d331bfc7
2021-10-08 13:39:41 -07:00
Zhichao Cao
bcd049cd2d Ingest external SST files with Temperature hints (#8949)
Summary:
Add the file temperature to `IngestExternalFileArg` such that when SST files are ingested, user is able to assign the temperature to each SST file. If the temperature vector is empty or its size does not match the file name vector size, all ingested SST files will be assigned with `Temperature::unKnown`.

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

Test Plan: add the new test and make check

Reviewed By: siying

Differential Revision: D31127852

Pulled By: zhichao-cao

fbshipit-source-id: 141a81f0f7b473d88f4ab0cb2a21a114cbc6f83c
2021-10-08 10:32:24 -07:00
anand76
2f1296ef48 Allow more flexible invocation of fbcode_stress_crash (#8985)
Summary:
Instead of hardcoding the stress test type and some args, allow it to be passed through env variables.

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

Reviewed By: akankshamahajan15

Differential Revision: D31349495

Pulled By: anand1976

fbshipit-source-id: 585c8fcb0232d0a95925b1a8c4e42a0940227e8b
2021-10-08 10:28:05 -07:00
Andrew Kryczka
c0ec58ecb9 stop populating unused/invalid MergingIterator heaps (#8975)
Summary:
I was looking at https://github.com/facebook/rocksdb/issues/2636 and got very confused that `MergingIterator::AddIterator()` is populating `min_heap_` with dangling pointers. There is justification in the comments that `min_heap_` will be cleared before it's used, but it'd be cleaner to not populate it with dangling pointers in the first place. Also made similar change in the constructor for consistency, although the pointers there would not be dangling, just unused.

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

Test Plan: rely on existing tests

Reviewed By: pdillinger, hx235

Differential Revision: D31273767

Pulled By: ajkr

fbshipit-source-id: 127ca9dd1f82f77f55dd0c3f19511de3282fc229
2021-10-07 15:26:08 -07:00
Andrew Kryczka
fcaa7ff638 Cancel manual compactions waiting on automatic compactions to drain (#8991)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8991

Test Plan: the new test hangs forever without this fix and passes with this fix.

Reviewed By: hx235

Differential Revision: D31456419

Pulled By: ajkr

fbshipit-source-id: a82c0e5560b6e6153089dccd8e46163c61b07bff
2021-10-07 15:23:55 -07:00
Kajetan Janiak
8717c26823 Warning about incompatible options with level_compaction_dynamic_level_bytes (#8329)
Summary:
This change introduces warnings instead of a silent override when trying to use level_compaction_dynamic_level_bytes with multiple cf_paths/db_paths.
I have completed the CLA.

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

Reviewed By: hx235

Differential Revision: D31399713

Pulled By: ajkr

fbshipit-source-id: 29c6fe5258d1f739b4590ecd44aee44f55415595
2021-10-07 15:23:55 -07:00
Zhichao Cao
b632ed0c67 Add file temperature related counter and bytes stats to and io_stats (#8710)
Summary:
For tiered storage project, we need to know the block read count and read bytes of files with different temperature. Add FileIOByTemperature to IOStatsContext and collect the bytes read and read count from different temperature files through the RandomAccessFileReader.

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

Test Plan: make check, add the testing cases

Reviewed By: siying

Differential Revision: D30582400

Pulled By: zhichao-cao

fbshipit-source-id: d83173de594374fc8404af5ce93a6a9be72c7141
2021-10-07 14:58:41 -07:00
Zhichao Cao
699f45049d Introduce a mechanism to dump out blocks from block cache and re-insert to secondary cache (#8912)
Summary:
Background: Cache warming up will cause potential read performance degradation due to reading blocks from storage to the block cache. Since in production, the workload and access pattern to a certain DB is stable, it is a potential solution to dump out the blocks belonging to a certain DB to persist storage (e.g., to a file) and bulk-load the blocks to Secondary cache before the DB is relaunched. For example, when migrating a DB form host A to host B, it will take a short period of time, the access pattern to blocks in the block cache will not change much. It is efficient to dump out the blocks of certain DB, migrate to the destination host and insert them to the Secondary cache before we relaunch the DB.

Design: we introduce the interface of CacheDumpWriter and CacheDumpRead for user to store the blocks dumped out from block cache. RocksDB will encode all the information and send the string to the writer. User can implement their own writer it they want. CacheDumper and CacheLoad are introduced to save the blocks and load the blocks respectively.

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

Test Plan: add new tests to lru_cache_test and pass make check.

Reviewed By: pdillinger

Differential Revision: D31452871

Pulled By: zhichao-cao

fbshipit-source-id: 11ab4f5d03e383f476947116361d54188d36ec48
2021-10-07 11:42:31 -07:00
Ramkumar Vadivelu
fe994bbd0b Misc doc fixes (#8983)
Summary:
- Update few stale GitHub wiki link references from rocksdb.org
- Update the API comments for ignore_range_deletions

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

Reviewed By: ajkr

Differential Revision: D31355965

Pulled By: ramvadiv

fbshipit-source-id: 245ac4a6913976dd82afa308bc4aae6bff3d788c
2021-10-07 11:22:17 -07:00
mrambacher
53e595d1f3 Cleanup multiple implementations of VectorIterator (#8901)
Summary:
There were three implementations of VectorIterator (util/vector_iterator, test_util/testutil.h and LoggingForwardVectorIterator).  Merged them into one class to increase code coverage/testing and reduce duplication.

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

Reviewed By: pdillinger

Differential Revision: D31022673

Pulled By: mrambacher

fbshipit-source-id: 8e3acbd2dfd60b4df609d02cc72846de2389d531
2021-10-06 07:48:31 -07:00
Yanqin Jin
d16ceba687 Point bzip2 download address to http://sourceware.org/pub/bzip2 (#8986)
Summary:
Download bzip2 from `https://sourceware.org/pub/bzip2` to `http://sourceware.org/pub/bzip2`
to resolve curl's ca verification error.

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

Reviewed By: akankshamahajan15

Differential Revision: D31387038

Pulled By: riversand963

fbshipit-source-id: 510fdb9530e63639cd5d20339f3f3cbf720068e9
2021-10-05 11:21:46 -07:00
Stefan Roesch
a776406de3 Add file operation callbacks to SequentialFileReader (#8982)
Summary:
This change adds File IO Notifications to the SequentialFileReader The SequentialFileReader is extended
with a listener parameter.

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

Test Plan:
A new test EventListenerTest::OnWALOperationTest has been added. The
test verifies that during restore the sequential file reader is called
and the notifications are fired.

Reviewed By: riversand963

Differential Revision: D31320844

Pulled By: shrfb

fbshipit-source-id: 040b24da7c010d7c14ebb5c6460fae9a19b8c168
2021-10-05 10:51:59 -07:00
mrambacher
787229837e Fix LITE mode builds on MacOs (#8981)
Summary:
On MacOS, there were errors building in LITE mode related to unused private member variables:

In file included from ./db/compaction/compaction_job.h:20:
./db/blob/blob_file_completion_callback.h:87:19: error: private field ‘sst_file_manager_’ is not used [-Werror,-Wunused-private-field]
  SstFileManager* sst_file_manager_;
                  ^
./db/blob/blob_file_completion_callback.h:88:22: error: private field ‘mutex_’ is not used [-Werror,-Wunused-private-field]
  InstrumentedMutex* mutex_;
                     ^
./db/blob/blob_file_completion_callback.h:89:17: error: private field ‘error_handler_’ is not used [-Werror,-Wunused-private-field]
  ErrorHandler* error_handler_;

This PR resolves those build issues by removing the values as members in LITE mode and fixing the constructor to ignore the input values in LITE mode (otherwise we get unused parameter warnings).

Tested by validating compiles without warnings.

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

Reviewed By: akankshamahajan15

Differential Revision: D31320141

Pulled By: mrambacher

fbshipit-source-id: d67875ebbd39a9555e4f09b2d37159566dd8a085
2021-10-04 05:30:26 -07:00
Yanqin Jin
2cdaf5ca5b Add additional checks for three existing unit tests (#8973)
Summary:
With test sync points, we can assert on the equality of iterator value in three existing
unit tests.

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

Test Plan:
```
gtest-parallel -r 1000 ./db_test2 --gtest_filter=DBTest2.IterRaceFlush2:DBTest2.IterRaceFlush1:DBTest2.IterRefreshRaceFlush
```

make check

Reviewed By: akankshamahajan15

Differential Revision: D31256340

Pulled By: riversand963

fbshipit-source-id: a9440767ab383e0ec61bd43ffa8fbec4ba562ea2
2021-10-01 17:22:37 -07:00
Akanksha Mahajan
84d71f30c4 Enable SingleDelete with user defined ts in db_bench and crash tests (#8971)
Summary:
Enable SingleDelete with user defined timestamp in db_bench,
db_stress and crash test

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

Test Plan:
1. For db_stress, ran the command for full duration: i) python3 -u tools/db_crashtest.py
--enable_ts whitebox --nooverwritepercent=100
ii) make crash_test_with_ts

2. For db_bench, ran:  ./db_bench -benchmarks=randomreplacekeys
-user_timestamp_size=8 -use_single_deletes=true

Reviewed By: riversand963

Differential Revision: D31246558

Pulled By: akankshamahajan15

fbshipit-source-id: 29cd8740c9921341e52f09242fca3c44d75a12b7
2021-10-01 16:48:01 -07:00
byronhe
e36b9da57c Update USERS.md (#8923)
Summary:
fix typo

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

Reviewed By: mrambacher

Differential Revision: D31003331

Pulled By: zhichao-cao

fbshipit-source-id: 00cfcac247621b8bc6d43a3d45c6a11c9dece5b0
2021-10-01 16:10:35 -07:00
sdong
7f08a8503f Remove IOSTATS_ADD_IF_POSITIVE() (#8984)
Summary:
IOSTATS_ADD_IF_POSITIVE() doesn't seem to a macro that aims to improve performance but does the opposite. The counter to add is almost always positive so the if is just a waste. Furthermore, adding to a thread local variable seemse to be much cheaper than an if condition if branch prediction has a possibility to be wrong. Remove the macro.

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

Test Plan: See CI completes.

Reviewed By: anand1976

Differential Revision: D31348163

fbshipit-source-id: 30af6d45e1aa8bbc09b2c046206cce6f67f4777a
2021-10-01 14:43:00 -07:00
Pradeep Ambati
e5bfb91d09 List blob files when using command - list_live_files_metadata (#8976)
Summary:
The ldb list_live_files_metadata command does not print any information about blob files currently. We would like to add this functionality. Note that list_live_files_metadata has two different modes of operation: the one shown above, which shows the LSM tree structure, and another one, which can be enabled using the flag --sort_by_filename and simply lists the files in numerical order regardless of level. We would like to show blob files in both modes.

Changes:
1. Using GetAllColumnFamilyMetaData API instead of GetLiveFilesMetaData API for fetching live files data.

Testing:
1. Created a sample rocksdb instance using dbbench command (this creates both SST and blob files)
2. Checked if the blob files are listed or not by using ldb commands.

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

Reviewed By: ltamasi

Differential Revision: D31316061

Pulled By: pradeepambati

fbshipit-source-id: d15cdea192febf7a45f28deee2ba40615d3d84ab
2021-09-30 15:13:11 -07:00
Peter (Stig) Edwards
1953b63cdd ErrorExit if num<1000 for fillsync and fill100K (#8391)
Summary:
This is to avoid an exception and core dump when running
  db_bench -benchmarks fillsync -num 999
https://github.com/facebook/rocksdb/issues/8390

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

Reviewed By: pdillinger

Differential Revision: D29139688

Pulled By: mrambacher

fbshipit-source-id: b9e306728ad25a7aac75f6154699aa852bc07bd1
2021-09-30 14:17:36 -07:00
anand76
532ff334d9 Don't ignore deletion rate limit if WAL dir is different (#8967)
Summary:
If WAL dir is different from the DB dir, we should still honor the SstFileManager deletion rate limit for SST files.

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

Test Plan: Add a new unit test in db_sst_test

Reviewed By: pdillinger

Differential Revision: D31220116

Pulled By: anand1976

fbshipit-source-id: bcde8a53a7d728e15e597fb5d07ee86c1b38bd28
2021-09-30 13:26:31 -07:00
Yanqin Jin
2acffecca1 Add comments for MultiGetBlob() and checks for MultiRead() (#8972)
Summary:
Add comments for MultiGetBlob() that input argument `offsets` must be
sorted. In addition, add assertion for this condition in debug build.
Repeat the same for RandomAccessFileReader::MultiRead().

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

Test Plan: make check

Reviewed By: pdillinger

Differential Revision: D31253205

Pulled By: riversand963

fbshipit-source-id: 98758229b8052f3aeb319d5584026b4de2d220a2
2021-09-29 14:27:19 -07:00
Peter Dillinger
61a63ae2f9 Add/improve misc comments (#8963)
Summary:
Fill in some missing info; fix some incorrect info.

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

Test Plan: comments only

Reviewed By: mrambacher

Differential Revision: D31211183

Pulled By: pdillinger

fbshipit-source-id: 783ff6673791c01d44c3ed92d4398c64ae5a5005
2021-09-29 11:20:53 -07:00
mrambacher
13ae16c315 Cleanup includes in dbformat.h (#8930)
Summary:
This header file was including everything and the kitchen sink when it did not need to.  This resulted in many places including this header when they needed other pieces instead.

Cleaned up this header to only include what was needed and fixed up the remaining code to include what was now missing.

Hopefully, this sort of code hygiene cleanup will speed up the builds...

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

Reviewed By: pdillinger

Differential Revision: D31142788

Pulled By: mrambacher

fbshipit-source-id: 6b45de3f300750c79f751f6227dece9cfd44085d
2021-09-29 04:04:40 -07:00
Andrew Kryczka
559943cdc0 Refactor expected state in stress/crash test (#8913)
Summary:
This is a precursor refactoring to enable an upcoming feature: persistence failure correctness testing.

- Changed `--expected_values_path` to `--expected_values_dir` and migrated "db_crashtest.py" to use the new flag. For persistence failure correctness testing there are multiple possible correct states since unsynced data is allowed to be dropped. Making it possible to restore all these possible correct states will eventually involve files containing snapshots of expected values and DB trace files.
- The expected values directory is managed by an `ExpectedStateManager` instance. Managing expected state files is separated out of `SharedState` to prevent `SharedState` from becoming too complex when the new files and features (snapshotting, tracing, and restoring) are introduced.
- Migrated expected values file access/management out of `SharedState` into a separate class called `ExpectedState`. This is not exposed directly to the test but rather the `ExpectedState` for the latest values file is accessed via a pass-through API on `ExpectedStateManager`. This forces the test to always access the single latest `ExpectedState`.
- Changed the initialization of the latest expected values file to use a tempfile followed by rename, and also add cleanup logic for possible stranded tempfiles.

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

Test Plan:
run in several ways; try to make sure it's not obviously broken.

- crashtest blackbox without TEST_TMPDIR
```
$ python3 tools/db_crashtest.py blackbox --simple --write_buffer_size=1048576 --target_file_size_base=1048576 --max_bytes_for_level_base=4194304 --max_key=100000 --value_size_mult=33 --compression_type=none --duration=120 --interval=10 --compression_type=none --blob_compression_type=none
```
- crashtest blackbox with TEST_TMPDIR
```
$ TEST_TMPDIR=/dev/shm python3 tools/db_crashtest.py blackbox --simple --write_buffer_size=1048576 --target_file_size_base=1048576 --max_bytes_for_level_base=4194304 --max_key=100000 --value_size_mult=33 --compression_type=none --duration=120 --interval=10 --compression_type=none --blob_compression_type=none
```
- crashtest whitebox with TEST_TMPDIR
```
$ TEST_TMPDIR=/dev/shm python3 tools/db_crashtest.py whitebox --simple --write_buffer_size=1048576 --target_file_size_base=1048576 --max_bytes_for_level_base=4194304 --max_key=100000 --value_size_mult=33 --compression_type=none --duration=120 --interval=10 --compression_type=none --blob_compression_type=none --random_kill_odd=88887
```
- db_stress without expected_values_dir
```
$ ./db_stress --write_buffer_size=1048576 --target_file_size_base=1048576 --max_bytes_for_level_base=4194304 --max_key=100000 --value_size_mult=33 --compression_type=none --ops_per_thread=10000 --clear_column_family_one_in=0 --destroy_db_initially=true
```
- db_stress with expected_values_dir and manual corruption
```
$ ./db_stress --write_buffer_size=1048576 --target_file_size_base=1048576 --max_bytes_for_level_base=4194304 --max_key=100000 --value_size_mult=33 --compression_type=none --ops_per_thread=10000 --clear_column_family_one_in=0 --destroy_db_initially=true --expected_values_dir=./
// modify one byte in "./LATEST.state"
$ ./db_stress --write_buffer_size=1048576 --target_file_size_base=1048576 --max_bytes_for_level_base=4194304 --max_key=100000 --value_size_mult=33 --compression_type=none --ops_per_thread=10000 --clear_column_family_one_in=0 --destroy_db_initially=false --expected_values_dir=./
...
Verification failed for column family 0 key 0000000000000000 (0): Value not found: NotFound:
...
```

Reviewed By: riversand963

Differential Revision: D30921951

Pulled By: ajkr

fbshipit-source-id: babfe218062e55d018c9b046536c0289fb78f41c
2021-09-28 14:13:33 -07:00
Jay Zhuang
6b34eb0ebc Add remote compaction read/write bytes statistics (#8939)
Summary:
Add basic read/write bytes statistics on the primary side:
`REMOTE_COMPACT_READ_BYTES`
`REMOTE_COMPACT_WRITE_BYTES`

Fixed existing statistics missing some IO for remote compaction.

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

Test Plan: CI

Reviewed By: ajkr

Differential Revision: D31074672

Pulled By: jay-zhuang

fbshipit-source-id: c57afdba369990185008ffaec7e3fe7c62e8902f
2021-09-28 14:00:37 -07:00
Hui Xiao
d6bd1a0291 Support "level_at_creation" in TablePropertiesCollectorFactory::Context (#8919)
Summary:
Context:
Exposing the level of the sst file (i.e, table) where it is created in `TablePropertiesCollectorFactory::Context` allows users of `TablePropertiesCollectorFactory` to customize some implementation details of `TablePropertiesCollectorFactory` and `TablePropertiesCollector` based on the level of creation. For example, `TablePropertiesCollector::NeedCompact()` can return different values based on level of creation.
- Declared an extra field `level_at_creation` in `TablePropertiesCollectorFactory::Context`
- Allowed `level_at_creation` to be passed in as an argument in `IntTblPropCollectorFactory::CreateIntTblPropCollector()` and `UserKeyTablePropertiesCollectorFactory::CreateIntTblPropCollector()`, the latter of which is an internal wrapper of user's passed-in `TablePropertiesCollectorFactory::CreateTablePropertiesCollector()` used in table-building process
- Called `IntTblPropCollectorFactory::CreateIntTblPropCollector()` with `level_at_creation` passed into both `BlockBasedTableBuilder` and `PlainTableBuilder`
  -  `PlainTableBuilder` previously did not capture `level_at_creation` from `TableBuilderOptions` in `PlainTableFactory`. In order for it to call the method with this parameter, this PR also made `PlainTableBuilder` capture `level_at_creation` as a required parameter
- Called `IntTblPropCollectorFactory::CreateIntTblPropCollector()` with `level_at_creation` its overridden functions in its derived classes, including `RegularKeysStartWithAFactory::CreateIntTblPropCollector()` in `table_properties_collector_test.cc`, `SstFileWriterPropertiesCollectorFactory::CreateIntTblPropCollector()` in `sst_file_writer_collectors.h`

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

Test Plan:
- Passed the added assertion for `context.level_at_creation`
- Passed existing tests
- Run `Make` to make sure adding a required parameter to `PlainTableBuilder`'s constructor does not break anything

Reviewed By: anand1976

Differential Revision: D30951729

Pulled By: hx235

fbshipit-source-id: c4a0173b0d9344a4cf47e1b987d759c1c73cb474
2021-09-28 12:35:24 -07:00
mrambacher
7fd68b7c39 Make WalFilter, SstPartitionerFactory, FileChecksumGenFactory, and TableProperties Customizable (#8638)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8638

Reviewed By: zhichao-cao

Differential Revision: D31024729

Pulled By: mrambacher

fbshipit-source-id: 954c04ccab0b8dee64050a27aadf78ed119106c0
2021-09-28 05:32:02 -07:00
sdong
b88109db19 Pollute buffer before calling Read() (#8955)
Summary:
Add a paranoid check where in case FileSystem layer doesn't fill the buffer but returns succeed, checksum is unlikely to match even if buffer contains a previous block. The byte modified is not useful anyway, so it isn't expect to change any behavior.

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

Test Plan: See existing CI to pass.

Reviewed By: pdillinger

Differential Revision: D31183966

fbshipit-source-id: dcc4de429e18131873f783b90d3be55d7eb44a1f
2021-09-27 21:30:28 -07:00
dependabot[bot]
345f4c9462 Bump nokogiri from 1.11.4 to 1.12.5 in /docs (#8965)
Summary:
Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.11.4 to 1.12.5.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/sparklemotion/nokogiri/releases">nokogiri's releases</a>.</em></p>
<blockquote>
<h2>1.12.5 / 2021-09-27</h2>
<h3>Security</h3>
<p>[JRuby] Address CVE-2021-41098 (<a href="https://github.com/sparklemotion/nokogiri/security/advisories/GHSA-2rr5-8q37-2w7h">GHSA-2rr5-8q37-2w7h</a>).</p>
<p>In Nokogiri v1.12.4 and earlier, on JRuby only, the SAX parsers resolve external entities (XXE) by default. This fix turns off entity-resolution-by-default in the JRuby SAX parsers to match the CRuby SAX parsers' behavior.</p>
<p>CRuby users are not affected by this CVE.</p>
<h3>Fixed</h3>
<ul>
<li>[CRuby] <code>Document#to_xhtml</code> properly serializes self-closing tags in libxml &gt; 2.9.10. A behavior change introduced in libxml 2.9.11 resulted in emitting start and and tags (e.g., <code>&lt;br&gt;&lt;/br&gt;</code>) instead of a self-closing tag (e.g., <code>&lt;br/&gt;</code>) in previous Nokogiri versions. [<a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2324">https://github.com/facebook/rocksdb/issues/2324</a>]</li>
</ul>
<hr />
<p>SHA256 checksums:</p>
<pre><code>36bfa3a07aced069b3f3c9b39d9fb62cb0728d284d02b079404cd55780beaeff  nokogiri-1.12.5-arm64-darwin.gem
16b1a9ddbb70a9c998462912a5972097cbc79c3e01eb373906886ef8a469f589  nokogiri-1.12.5-java.gem
218dcc6edd1b49cc6244b5f88afb978739bb2f3f166c271557fe5f51e4bc713c  nokogiri-1.12.5-x64-mingw32.gem
e33bb919d64c16d931a5f26dc880969e587d225cfa97e6b56e790fb52179f527  nokogiri-1.12.5-x86-linux.gem
e13c2ed011b8346fbd589e96fe3542d763158bc2c7ad0f4f55f6d801afd1d9ff  nokogiri-1.12.5-x86-mingw32.gem
1ed64f7db7c1414b87fce28029f2a10128611d2037e0871ba298d00f9a00edd6  nokogiri-1.12.5-x86_64-darwin.gem
0868c8d0a147904d4dedaaa05af5f06656f2d3c67e4432601718559bf69d6cea  nokogiri-1.12.5-x86_64-linux.gem
2b20905942acc580697c8c496d0d1672ab617facb9d30d156b3c7676e67902ec  nokogiri-1.12.5.gem
</code></pre>
<h2>1.12.4 / 2021-08-29</h2>
<h3>Notable fix: Namespace inheritance</h3>
<p>Namespace behavior when reparenting nodes has historically been poorly specified and the behavior diverged between CRuby and JRuby. As a result, making this behavior consistent in v1.12.0 introduced a breaking change.</p>
<p>This patch release reverts the Builder behavior present in v1.12.0..v1.12.3 but keeps the Document behavior. This release also introduces a Document attribute to allow affected users to easily change this behavior for their legacy code without invasive changes.</p>
<h4>Compensating Feature in XML::Document</h4>
<p>This release of Nokogiri introduces a new <code>Document</code> boolean attribute, <code>namespace_inheritance</code>, which controls whether children should inherit a namespace when they are reparented. <code>Nokogiri::XML:Document</code> defaults this attribute to <code>false</code> meaning &quot;do not inherit,&quot; thereby making explicit the behavior change introduced in v1.12.0.</p>
<p>CRuby users who desire the pre-v1.12.0 behavior may set <code>document.namespace_inheritance = true</code> before reparenting nodes.</p>
<p>See <a href="https://nokogiri.org/rdoc/Nokogiri/XML/Document.html#namespace_inheritance-instance_method">https://nokogiri.org/rdoc/Nokogiri/XML/Document.html#namespace_inheritance-instance_method</a> for example usage.</p>
<h4>Fix for XML::Builder</h4>

</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md">nokogiri's changelog</a>.</em></p>
<blockquote>
<h2>1.12.5 / 2021-09-27</h2>
<h3>Security</h3>
<p>[JRuby] Address CVE-2021-41098 (<a href="https://github.com/sparklemotion/nokogiri/security/advisories/GHSA-2rr5-8q37-2w7h">GHSA-2rr5-8q37-2w7h</a>).</p>
<p>In Nokogiri v1.12.4 and earlier, on JRuby only, the SAX parsers resolve external entities (XXE) by default. This fix turns off entity-resolution-by-default in the JRuby SAX parsers to match the CRuby SAX parsers' behavior.</p>
<p>CRuby users are not affected by this CVE.</p>
<h3>Fixed</h3>
<ul>
<li>[CRuby] <code>Document#to_xhtml</code> properly serializes self-closing tags in libxml &gt; 2.9.10. A behavior change introduced in libxml 2.9.11 resulted in emitting start and and tags (e.g., <code>&lt;br&gt;&lt;/br&gt;</code>) instead of a self-closing tag (e.g., <code>&lt;br/&gt;</code>) in previous Nokogiri versions. [<a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2324">https://github.com/facebook/rocksdb/issues/2324</a>]</li>
</ul>
<h2>1.12.4 / 2021-08-29</h2>
<h3>Notable fix: Namespace inheritance</h3>
<p>Namespace behavior when reparenting nodes has historically been poorly specified and the behavior diverged between CRuby and JRuby. As a result, making this behavior consistent in v1.12.0 introduced a breaking change.</p>
<p>This patch release reverts the Builder behavior present in v1.12.0..v1.12.3 but keeps the Document behavior. This release also introduces a Document attribute to allow affected users to easily change this behavior for their legacy code without invasive changes.</p>
<h4>Compensating Feature in XML::Document</h4>
<p>This release of Nokogiri introduces a new <code>Document</code> boolean attribute, <code>namespace_inheritance</code>, which controls whether children should inherit a namespace when they are reparented. <code>Nokogiri::XML:Document</code> defaults this attribute to <code>false</code> meaning &quot;do not inherit,&quot; thereby making explicit the behavior change introduced in v1.12.0.</p>
<p>CRuby users who desire the pre-v1.12.0 behavior may set <code>document.namespace_inheritance = true</code> before reparenting nodes.</p>
<p>See <a href="https://nokogiri.org/rdoc/Nokogiri/XML/Document.html#namespace_inheritance-instance_method">https://nokogiri.org/rdoc/Nokogiri/XML/Document.html#namespace_inheritance-instance_method</a> for example usage.</p>
<h4>Fix for XML::Builder</h4>
<p>However, recognizing that we want <code>Builder</code>-created children to inherit namespaces, Builder now will set <code>namespace_inheritance=true</code> on the underlying document for both JRuby and CRuby. This means that, on CRuby, the pre-v1.12.0 behavior is restored.</p>
<p>Users who want to turn this behavior off may pass a keyword argument to the Builder constructor like so:</p>
<pre lang="ruby"><code>Nokogiri::XML::Builder.new(namespace_inheritance: false)
</code></pre>
<p>See <a href="https://nokogiri.org/rdoc/Nokogiri/XML/Builder.html#label-Namespace+inheritance">https://nokogiri.org/rdoc/Nokogiri/XML/Builder.html#label-Namespace+inheritance</a> for example usage.</p>
<h4>Downstream gem maintainers</h4>
<p>Note that any downstream gems may want to specifically omit Nokogiri v1.12.0--v1.12.3 from their dependency specification if they rely on child namespace inheritance:</p>

</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="47f6a461fd"><code>47f6a46</code></a> version bump to v1.12.5</li>
<li><a href="2a0ac88518"><code>2a0ac88</code></a> update CHANGELOG</li>
<li><a href="6b6063782c"><code>6b60637</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2329">https://github.com/facebook/rocksdb/issues/2329</a> from sparklemotion/flavorjones-GHSA-2rr5-8q37-2w7h_1...</li>
<li><a href="4bd943cae3"><code>4bd943c</code></a> fix(jruby): SAX parser uses an entity resolver</li>
<li><a href="f943ee4108"><code>f943ee4</code></a> refactor(jruby): handle errors more consistently</li>
<li><a href="2790122748"><code>2790122</code></a> format: test files</li>
<li><a href="01e1618f75"><code>01e1618</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2327">https://github.com/facebook/rocksdb/issues/2327</a> from sparklemotion/2324-xhtml-self-closing-tags_v1.12.x</li>
<li><a href="a0180c72c5"><code>a0180c7</code></a> fix: HTML4::Document.to_xhtml self-closing tags</li>
<li><a href="564ac17873"><code>564ac17</code></a> release v1.12.4</li>
<li><a href="4d5754baed"><code>4d5754b</code></a> backport <a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2320">https://github.com/facebook/rocksdb/issues/2320</a></li>
<li>Additional commits viewable in <a href="https://github.com/sparklemotion/nokogiri/compare/v1.11.4...v1.12.5">compare view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=nokogiri&package-manager=bundler&previous-version=1.11.4&new-version=1.12.5)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

 ---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `dependabot rebase` will rebase this PR
- `dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `dependabot merge` will merge this PR after your CI passes on it
- `dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `dependabot cancel merge` will cancel a previously requested merge and block automerging
- `dependabot reopen` will reopen this PR if it is closed
- `dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language

You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/facebook/rocksdb/network/alerts).

</details>

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

Reviewed By: akankshamahajan15

Differential Revision: D31217632

Pulled By: ltamasi

fbshipit-source-id: c98c5a42f29eb45164a266edd91569737595ab2a
2021-09-27 15:21:05 -07:00
Akanksha Mahajan
a2f29ce70a Update History.md for SingleDelete with user defined timestamp (#8964)
Summary:
Update History.md for SingleDelete with user defined timestamp

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

Reviewed By: zhichao-cao

Differential Revision: D31216214

Pulled By: akankshamahajan15

fbshipit-source-id: 0520132c75fe8f6823d154e41585b0df3086c04d
2021-09-27 14:58:30 -07:00
Akanksha Mahajan
78afb4d81e Support SingleDelete for user-defined timestamps (#8921)
Summary:
Added support for SingleDelete for user-defined timestamps. Users can now Get and Iterate over keys deleted with SingleDelete. It also includes changes in CompactionIterator which  preserves the same user key with different timestamps, unless the timestamp is below a certain threshold full_history_ts_low.

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

Test Plan: Added new unit tests

Reviewed By: riversand963

Differential Revision: D31098191

Pulled By: akankshamahajan15

fbshipit-source-id: 78a59ef4b4884ae324fcd10f56e62a27d5ee2f49
2021-09-27 11:51:07 -07:00
Peter Dillinger
0774d640c0 Fix some lint warnings reported on 6.25 (#8945)
Summary:
Fix some lint warnings

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

Test Plan: existing tests, linters

Reviewed By: zhichao-cao

Differential Revision: D31103824

Pulled By: pdillinger

fbshipit-source-id: 4dd9b0c30fa50e588107ac6ed392b2dfb507a5d4
2021-09-27 11:43:20 -07:00
ricky
b59b7570cf More clear error message on uncompressing block (#8934)
Summary:
The origin error message of uncompressing block is confusing, which may result from either build support or data corruption.

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

Reviewed By: ltamasi

Differential Revision: D31112588

Pulled By: pdillinger

fbshipit-source-id: 1cbf2d4fbcb0ef376cf942246d06f48cb603f852
2021-09-27 10:38:16 -07:00
mrambacher
e0f697d2bd Make SliceTransform into a Customizable class (#8641)
Summary:
Made SliceTransform into a Customizable class.

Would be nice to write a test that stored and used a custom transform  in an SST table.

There are a set of tests (DBBlockFliterTest.PrefixExtractor*, SamePrefixTest.InDomainTest, PrefixTest.PrefixAndWholeKeyTest that run the same with or without a SliceTransform/PrefixFilter.  Is this expected?

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

Reviewed By: zhichao-cao

Differential Revision: D31142793

Pulled By: mrambacher

fbshipit-source-id: bb08672fccbfdc263dcae21f25a62307e1facda1
2021-09-27 07:43:47 -07:00
Yanqin Jin
b92cef2d1d Sort per-file blob read requests by offset (#8953)
Summary:
`RandomAccessFileReader::MultiRead()` tries to merge requests in direct IO, assuming input IO requests are
sorted by offsets.

Add a test in direct IO mode.

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

Test Plan: make check

Reviewed By: ltamasi

Differential Revision: D31183546

Pulled By: riversand963

fbshipit-source-id: 5d043ec68e2daa47a3149066150afd41ee3d73e6
2021-09-24 22:14:30 -07:00
Andrew Kryczka
6d424be910 Temporarily set experimental_mempurge_threshold=0 in crash test (#8958)
Summary:
For now, disable it since the below command indicates it can cause a
failure. Running that command with `-experimental_mempurge_threshold=0`
has been running successfully for several minutes, whereas before it
failed in seconds.

```
$ while rm -rf /dev/shm/single_stress && ./db_stress --clear_column_family_one_in=0 --column_families=1 --db=/dev/shm/single_stress --experimental_mempurge_threshold=5.493146827397074 --flush_one_in=10000 --reopen=0 --write_buffer_size=262144 --value_size_mult=33 --max_write_buffer_number=3 -ops_per_thread=10000; do : ; done
```

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

Reviewed By: ltamasi

Differential Revision: D31187059

Pulled By: ajkr

fbshipit-source-id: 04d5bfb4fcc4f5b66233e691427dfd940c67037f
2021-09-24 18:29:48 -07:00
Andrew Kryczka
791bff5b4e Prevent deadlock in db_stress with DbStressCompactionFilter (#8956)
Summary:
The cyclic dependency was:

- `StressTest::OperateDb()` locks the mutex for key 'k'
- `StressTest::OperateDb()` calls a function like `PauseBackgroundWork()`, which waits for pending compaction to complete.
- The pending compaction reaches key `k` and `DbStressCompactionFilter::FilterV2()` calls `Lock()` on that key's mutex, which hangs forever.

The cycle can be broken by using a new function, `port::Mutex::TryLock()`, which returns immediately upon failure to acquire a lock. In that case `DbStressCompactionFilter::FilterV2()` can just decide to keep the key.

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

Reviewed By: riversand963

Differential Revision: D31183718

Pulled By: ajkr

fbshipit-source-id: 329e4a31ce43085af174cf367ef560b5a04399c5
2021-09-24 16:54:02 -07:00
Jay Zhuang
0595101c4f Add microbench build support for fbcode (#8954)
Summary:
For internal build enviroment only. Developer could run the
microbenchmark without `ROCKSDB_NO_FBCODE=1`.

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

Test Plan: `$ make microbench` on dev server

Reviewed By: ajkr

Differential Revision: D31163717

Pulled By: jay-zhuang

fbshipit-source-id: 1ff59f660ca05afd0fd5c7c7dcdfd831ac365462
2021-09-24 10:23:35 -07:00
sdong
7c6a7e8fa8 FaultInjectionTestFS::InjectThreadSpecificReadError() should not corrupt mmaped bytes (#8952)
Summary:
Right now FaultInjectionTestFS::InjectThreadSpecificReadError() might try to corrupt return bytes, but these bytes might be from mmapped files, which would cause segfault. Instead FaultInjectionTestFS::InjectThreadSpecificReadError() should never corrupt data unless it is in caller's buffer.

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

Test Plan: See db_stress still runs and make sure in a test run failurs are still injected in non-mmap cases.

Reviewed By: ajkr, ltamasi

Differential Revision: D31147318

fbshipit-source-id: 9484a64ff2aaa36685557203f449286e694e65f9
2021-09-23 12:00:47 -07:00
Hui Xiao
b25f2afeff Return Status::NotSupported() in RateLimiter::GetTotalPendingRequests default impl (#8950)
Summary:
Context:
After more discussion, a fix in https://github.com/facebook/rocksdb/issues/8938 might turn out to be too restrictive for the case where `GetTotalPendingRequests` might be invoked on RateLimiter classes that does not support the recently added API `RateLimiter::GetTotalPendingRequests` (https://github.com/facebook/rocksdb/issues/8890) due to the `assert(false)` in https://github.com/facebook/rocksdb/issues/8938. Furthermore, sentinel value like `-1` proposed in https://github.com/facebook/rocksdb/issues/8938 is easy to be ignored and unchecked. Therefore we decided to adopt `Status::NotSupported()`, which is also a convention of adding new API to public header in RocksDB.
- Changed return value type of  `RateLimiter::GetTotalPendingRequests` in related declaration/definition
- Passed in pointer argument to hold the output instead of returning it as before
- Adapted to the changes above in calling `RateLimiter::GetTotalPendingRequests` in test
- Minor improvement to `TEST_F(RateLimiterTest, GetTotalPendingRequests)`:  added failure message for assertion and replaced repetitive statements with a loop

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

Reviewed By: ajkr, pdillinger

Differential Revision: D31128450

Pulled By: hx235

fbshipit-source-id: 282ac9c4f3dacaa0aec6d0a993161f77ad47a040
2021-09-22 19:36:06 -07:00
Levi Tamasi
be206db351 Deflake MySQLStyleTransactionTest.TransactionStressTest in "status checked" mode (#8947)
Summary:
There is a corner case when using WriteUnprepared transactions when
`WriteUnpreparedTxn::Get` returns `Status::TryAgain` instead of
propagating the result of `GetFromBatchAndDB`. The patch adds
`PermitUncheckedError` to make the `ASSERT_STATUS_CHECKED` build pass in
this case as well.

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

Test Plan: `make check`

Reviewed By: riversand963

Differential Revision: D31125422

Pulled By: ltamasi

fbshipit-source-id: 42de51dcfa9384e032244c2b4d3f40e9a4111194
2021-09-22 16:40:25 -07:00
sdong
c988e4720b Add HISTORY.md entry to a recent bug fix. (#8948)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8948

Reviewed By: anand1976

Differential Revision: D31127368

fbshipit-source-id: a374cb0baf88c3e15cd587a8f31e8a2d84432928
2021-09-22 16:23:08 -07:00