Commit graph

9924 commits

Author SHA1 Message Date
Akanksha Mahajan 83031e7343 Fix for LITE mode failure on MacOS (#8189)
Summary:
Fix for failure to build in LITE mode on MacOs from
BlobFileCompletionCallback unused private fields.

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

Reviewed By: jay-zhuang

Differential Revision: D27768341

Pulled By: akankshamahajan15

fbshipit-source-id: 14d31d7a9b52d308d9f9f27feff1977c5550622f
2021-04-15 09:45:02 -07:00
Akanksha Mahajan 296b47db25 Extend file_checksum_dump ldb command and DB::GetLiveFilesChecksumInfo to blob files (#8179)
Summary:
Extend the DB::GetLiveFilesChecksumInfo API to blob files.
This API is also used by the file_checksum_dump ldb command to dump checksum
of SST files which now also dumps blob files checksum.

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

Test Plan: Add new unit test

Reviewed By: zhichao-cao

Differential Revision: D27714965

Pulled By: akankshamahajan15

fbshipit-source-id: d8b7343ea845a64c83800336d88cced7152a8c92
2021-04-15 09:38:13 -07:00
Yanqin Jin b1f62be10e Use the right level (L0) for files written during WAL recovery (#8187)
Summary:
As the name of `DBImpl::WriteLevel0TableForRecovery` suggests, the resulting table file
should be placed on L0. However, the argument `level` passed to `BuildTable()` is -1.

We need to correct this since the level information will be useful to determine file placement.

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

Test Plan: make check

Reviewed By: ltamasi

Differential Revision: D27748570

Pulled By: riversand963

fbshipit-source-id: e1cd23128a8de31f14b1edc2ea92754c154e4f10
2021-04-14 23:40:22 -07:00
Justin Chapman d89483098f Assert unlimited max_open_files for FIFO compaction. (#8172)
Summary:
Resolves https://github.com/facebook/rocksdb/issues/8014

- Add an assertion on `DB::Open` to ensure `db_options.max_open_files` is unlimited if FIFO Compaction is being used.
- This is to align with what the docs mention and to prevent premature data deletion.
- Update tests to work with this assertion.

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

Test Plan:
```bash
$ make check -j$(nproc)

Generated TARGETS Summary:
- 6 libs
- 0 binarys
- 180 tests
```

Reviewed By: ajkr

Differential Revision: D27768792

Pulled By: thejchap

fbshipit-source-id: cf6350535e3a3577fec72bcba75b3c094dc7a6f3
2021-04-14 12:05:47 -07:00
sdong c861fb390d Add Blog Post "(Call For Contribution) Make Universal Compaction More Incremental" (#8182)
Summary:
Add a blog post that calls for contribution in incremental compaction

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

Reviewed By: ajkr

Differential Revision: D27724150

fbshipit-source-id: 42e474858b286a53e5aaa1c4e7242a8c745af651
2021-04-13 13:18:47 -07:00
Yanqin Jin fd00f39f97 Disable IOStatsContext/PerfContext if no thread local (#8117)
Summary:
Before this PR, `get_iostats_context()` will silently return a nullptr if no thread_local support is detected.
This can be the result of build_detect_platform's failure to compile the simple code snippet on certain platforms, as
reported in https://github.com/facebook/mysql-5.6/issues/904.
To be safe, we should fail the compilation if user does not opt out IOStatsContext and
ROCKSDB_SUPPORT_THREAD_LOCAL is not defined.

If RocksDB relies on c++11, can we just always use thread_local? It turns out there might be
performance concerns (https://github.com/facebook/rocksdb/issues/5774),
which is beyond the scope of this PR. We can revisit this later. Here, we stick to the original impl.

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

Reviewed By: ajkr

Differential Revision: D27356847

Pulled By: riversand963

fbshipit-source-id: f7d5776842277598d8341b955febb601946801ae
2021-04-13 07:56:59 -07:00
Peter Dillinger bb75092574 Misc Backup API enhancements (#8170)
Summary:
* CreateNewBackup(WithMetadata) returning the BackupID of new backup
through optional new output param. This is especially useful with the
new mutithreading support, so that you can transactionally determine the
ID of a backup you create.
* GetBackupInfo / GetLatestBackupInfo for individual backups, so that
you don't have to comb through a vector of backups if you don't want to.

Updated HISTORY.md (including re: BlobDB support as new feature)

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

Test Plan:
Added test logic to existing tests, to minimize increase in
cost of running tests

Reviewed By: zhichao-cao

Differential Revision: D27680410

Pulled By: pdillinger

fbshipit-source-id: 1fc45b73d81aae293ccd4a43d9583d7fd915d3eb
2021-04-12 11:00:47 -07:00
Xavier Deguillard 8972dd1ffa Add util/crc32c_arm64.cc to TARGETS (#8168)
Summary:
When compiling RocksDB with Buck for ARM64, the linker complains about missing crc32 symbols that are defined in the crc32c_arm64.cc file. Since this file wasn't included in the build this is totally expected

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

Test Plan:
The following no longer fails to link rocksdb:
  buck build mode/mac-xcode //eden/fs/service:edenfs#macosx-arm64

Reviewed By: zhichao-cao

Differential Revision: D27664627

Pulled By: xavierd

fbshipit-source-id: fb9d7a538599ee7a08882f87628731de6e641f8d
2021-04-12 10:57:56 -07:00
Sahir Hoda 139778dfb3 Expose Cache::DisownData in C API (#8160)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8160

Reviewed By: riversand963

Differential Revision: D27672474

Pulled By: ajkr

fbshipit-source-id: fdbbc3398f0b1d4cef6b68636e5caf369c34b3a7
2021-04-09 10:39:11 -07:00
David Carlier 728e5f5750 db_bench_tool: basic sys infos for FreeBSD. (#8169)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8169

Reviewed By: riversand963

Differential Revision: D27672457

Pulled By: ajkr

fbshipit-source-id: b40a7ad5d09a754154f28c2574ef9f77c8a131bb
2021-04-09 10:37:01 -07:00
Giuseppe Ottaviano 48cd7a3aae Fix flush reason attribution (#8150)
Summary:
Current flush reason attribution is misleading or incorrect (depending on what the original intention was):

- Flush due to WAL reaching its maximum size is attributed to `kWriteBufferManager`
- Flushes due to full write buffer and write buffer manager are not distinguishable, both are attributed to `kWriteBufferFull`

This changes the first to a new flush reason `kWALFull`, and splits the second between `kWriteBufferManager` and `kWriteBufferFull`.

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

Reviewed By: zhichao-cao

Differential Revision: D27569645

Pulled By: ot

fbshipit-source-id: 7e3c8ca186a6e71976e6b8e937297eebd4b769cc
2021-04-07 23:18:37 -07:00
Akanksha Mahajan 0be89e87fd Enable backup/restore for Integrated BlobDB in stress and crash tests (#8165)
Summary:
Enable backup/restore functionality with Integrated BlobDB in
db_stress and crash test.

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

Test Plan:
Ran python3 -u tools/db_crashtest.py --simple whitebox along
with :
  1. decreased "backup_in_one" value for backups to be more frequent and
  2. manually changed code for "enable_blob_file" to be always true and
     apply blobdb params 100% for testing purpose.

Reviewed By: ltamasi

Differential Revision: D27636025

Pulled By: akankshamahajan15

fbshipit-source-id: 0d0e0d1479ced163f992872dc998e79c581bfc99
2021-04-07 17:57:24 -07:00
Akanksha Mahajan d52b520d51 Integrated BlobDB for backup/restore support (#8129)
Summary:
Add support for blob files for backup/restore like table files.
    Since DB session ID is currently not supported for blob files (there is no place to store it in
    the header), so for blob files uses the
    kLegacyCrc32cAndFileSize naming scheme even if
    share_files_with_checksum_naming is set to kUseDbSessionId.

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

Test Plan: Add new test units

Reviewed By: ltamasi

Differential Revision: D27408510

Pulled By: akankshamahajan15

fbshipit-source-id: b27434d189a639ef3e6ad165c61a143a2daaf06e
2021-04-07 13:38:54 -07:00
Peter Dillinger a4e82a3cca Fix read-only DB writing to filesystem with write_dbid_to_manifest (#8164)
Summary:
Fixing another crash test failure in the case of
write_dbid_to_manifest=true and reading a backup as read-only DB.

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

Test Plan:
enhanced unit test for backup as read-only DB, ran
blackbox_crash_test more with elevated backup_one_in

Reviewed By: zhichao-cao

Differential Revision: D27622237

Pulled By: pdillinger

fbshipit-source-id: 680d0f99ddb465a601737f2e3f2c80efd47384fb
2021-04-07 10:26:47 -07:00
Peter Dillinger 35af0433cf Fix crash test with backup as read-only DB (#8161)
Summary:
Forgot to re-test crash test after adding read-only filesystem
enforcement to https://github.com/facebook/rocksdb/issues/8142. The problem is ReadOnlyFileSystem would reject
CreateDirIfMissing whenever DBOptions::create_if_missing=true. The fix
that is better for users is to allow CreateDirIfMissing in
ReadOnlyFileSystem if the directory exists, so that they don't cause a
failure on using create_if_missing with opening backups as read-only
DBs. Added this option test to the unit test (in addition to being in the
crash test).

Also fixed a couple of lints.

And some better messaging from 'make format' so that when you run it
with uncommitted changes, it's clear that it's only checking the
uncommitted changes.

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

Test Plan: local blackbox_crash_test with amplified backup_one_in

Reviewed By: ajkr

Differential Revision: D27614409

Pulled By: pdillinger

fbshipit-source-id: 63ccb626c7e34c200d61c6bca2a8f60da9015179
2021-04-06 23:31:51 -07:00
Sahir Hoda 6db3af1124 Update installation instructions (#8158)
Summary:
Updated instructions for installing zstd on CentOS and note about clang-format dependency

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

Reviewed By: riversand963

Differential Revision: D27598665

Pulled By: ajkr

fbshipit-source-id: e349eeb91147f3163e170cc29c8460b06d739b5b
2021-04-06 16:02:04 -07:00
Peter Dillinger 879357fdb0 Make backups openable as read-only DBs (#8142)
Summary:
A current limitation of backups is that you don't know the
exact database state of when the backup was taken. With this new
feature, you can at least inspect the backup's DB state without
restoring it by opening it as a read-only DB.

Rather than add something like OpenAsReadOnlyDB to the BackupEngine API,
which would inhibit opening stackable DB implementations read-only
(if/when their APIs support it), we instead provide a DB name and Env
that can be used to open as a read-only DB.

Possible follow-up work:

* Add a version of GetBackupInfo for a single backup.
* Let CreateNewBackup return the BackupID of the newly-created backup.

Implementation details:

Refactored ChrootFileSystem to split off new base class RemapFileSystem,
which allows more general remapping of files. We use this base class to
implement BackupEngineImpl::RemapSharedFileSystem.

To minimize API impact, I decided to just add these fields `name_for_open`
and `env_for_open` to those set by GetBackupInfo when
include_file_details=true. Creating the RemapSharedFileSystem adds a bit
to the memory consumption, perhaps unnecessarily in some cases, but this
has been mitigated by (a) only initialize the RemapSharedFileSystem
lazily when GetBackupInfo with include_file_details=true is called, and
(b) using the existing `shared_ptr<FileInfo>` objects to hold most of the
mapping data.

To enhance API safety, RemapSharedFileSystem is wrapped by new
ReadOnlyFileSystem which rejects any attempts to write. This uncovered a
couple of places in which DB::OpenForReadOnly would write to the
filesystem, so I fixed these. Added a release note because this affects
logging.

Additional minor refactoring in backupable_db.cc to support the new
functionality.

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

Test Plan:
new test (run with ASAN and UBSAN), added to stress test and
ran it for a while with amplified backup_one_in

Reviewed By: ajkr

Differential Revision: D27535408

Pulled By: pdillinger

fbshipit-source-id: 04666d310aa0261ef6b2385c43ca793ce1dfd148
2021-04-06 14:37:53 -07:00
Yanqin Jin 09528f9fa1 Fix a bug for SeekForPrev with partitioned filter and prefix (#8137)
Summary:
According to https://github.com/facebook/rocksdb/issues/5907, each filter partition "should include the bloom of the prefix of the last
key in the previous partition" so that SeekForPrev() in prefix mode can return correct result.
The prefix of the last key in the previous partition does not necessarily have the same prefix
as the first key in the current partition. Regardless of the first key in current partition, the
prefix of the last key in the previous partition should be added. The existing code, however,
does not follow this. Furthermore, there is another issue: when finishing current filter partition,
`FullFilterBlockBuilder::AddPrefix()` is called for the first key in next filter partition, which effectively
overwrites `last_prefix_str_` prematurely. Consequently, when the filter block builder proceeds
to the next partition, `last_prefix_str_` will be the prefix of its first key, leaving no way of adding
the bloom of the prefix of the last key of the previous partition.

Prefix extractor is FixedLength.2.
```
[  filter part 1   ]    [  filter part 2    ]
                  abc    d
```
When SeekForPrev("abcd"), checking the filter partition will land on filter part 2 because "abcd" > "abc"
but smaller than "d".
If the filter in filter part 2 happens to return false for the test for "ab", then SeekForPrev("abcd") will build
incorrect iterator tree in non-total-order mode.

Also fix a unit test which starts to fail following this PR. `InDomain` should not fail due to assertion
error when checking on an arbitrary key.

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

Test Plan:
```
make check
```

Without this fix, the following command will fail pretty soon.
```
./db_stress --acquire_snapshot_one_in=10000 --avoid_flush_during_recovery=0 \
--avoid_unnecessary_blocking_io=0 --backup_max_size=104857600 --backup_one_in=0 \
--batch_protection_bytes_per_key=0 --block_size=16384 --bloom_bits=17 \
--bottommost_compression_type=disable --cache_index_and_filter_blocks=1 --cache_size=1048576 \
--checkpoint_one_in=0 --checksum_type=kxxHash64 --clear_column_family_one_in=0 \
--compact_files_one_in=1000000 --compact_range_one_in=1000000 --compaction_ttl=0 \
--compression_max_dict_buffer_bytes=0 --compression_max_dict_bytes=0 \
--compression_parallel_threads=1 --compression_type=zstd --compression_zstd_max_train_bytes=0 \
--continuous_verification_interval=0 --db=/dev/shm/rocksdb/rocksdb_crashtest_whitebox \
--db_write_buffer_size=8388608 --delpercent=5 --delrangepercent=0 --destroy_db_initially=0 --enable_blob_files=0 \
--enable_compaction_filter=0 --enable_pipelined_write=1 --file_checksum_impl=big --flush_one_in=1000000 \
--format_version=5 --get_current_wal_file_one_in=0 --get_live_files_one_in=1000000 --get_property_one_in=1000000 \
--get_sorted_wal_files_one_in=0 --index_block_restart_interval=4 --index_type=2 --ingest_external_file_one_in=0 \
--iterpercent=10 --key_len_percent_dist=1,30,69 --level_compaction_dynamic_level_bytes=True \
--log2_keys_per_lock=10 --long_running_snapshots=1 --mark_for_compaction_one_file_in=0 \
--max_background_compactions=20 --max_bytes_for_level_base=10485760 --max_key=100000000 --max_key_len=3 \
--max_manifest_file_size=1073741824 --max_write_batch_group_size_bytes=16777216 --max_write_buffer_number=3 \
--max_write_buffer_size_to_maintain=8388608 --memtablerep=skip_list --mmap_read=1 --mock_direct_io=False \
--nooverwritepercent=0 --open_files=500000 --ops_per_thread=20000000 --optimize_filters_for_memory=0 --paranoid_file_checks=1 --partition_filters=1 --partition_pinning=0 --pause_background_one_in=1000000 \
--periodic_compaction_seconds=0 --prefixpercent=5 --progress_reports=0 --read_fault_one_in=0 --read_only=0 \
--readpercent=45 --recycle_log_file_num=0 --reopen=20 --secondary_catch_up_one_in=0 \
--snapshot_hold_ops=100000 --sst_file_manager_bytes_per_sec=104857600 \
--sst_file_manager_bytes_per_truncate=0 --subcompactions=2 --sync=0 --sync_fault_injection=False \
--target_file_size_base=2097152 --target_file_size_multiplier=2 --test_batches_snapshots=0 --test_cf_consistency=0 \
--top_level_index_pinning=0 --unpartitioned_pinning=1 --use_blob_db=0 --use_block_based_filter=0 \
--use_direct_io_for_flush_and_compaction=0 --use_direct_reads=0 --use_full_merge_v1=0 --use_merge=0 \
--use_multiget=0 --use_ribbon_filter=0 --use_txn=0 --user_timestamp_size=8 --verify_checksum=1 \
--verify_checksum_one_in=1000000 --verify_db_one_in=100000 --write_buffer_size=4194304 \
--write_dbid_to_manifest=1 --writepercent=35
```

Reviewed By: pdillinger

Differential Revision: D27553054

Pulled By: riversand963

fbshipit-source-id: 60e391e4a2d8d98a9a3172ec5d6176b90ec3de98
2021-04-06 12:14:08 -07:00
sunby c4d0e66d65 Remove check for status returned by InvalidatePageCache (#8156)
Summary:
Failures in `InvalidatePageCache` will change the API contract. So we remove the status check for `InvalidatePageCache` in `SstFileWriter::Add()`, `SstFileWriter::Finish` and `Rep::DeleteRange`

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

Reviewed By: riversand963

Differential Revision: D27597012

Pulled By: ajkr

fbshipit-source-id: 2872051695d50cc47ed0f2848dc582464c00076f
2021-04-06 11:55:14 -07:00
Yanqin Jin 2d8518f5ea Reset pinnable slice before using it in Get() (#8154)
Summary:
Fixes https://github.com/facebook/rocksdb/issues/6548.
If we do not reset the pinnable slice before calling get, we will see the following assertion failure
while running the test with multiple column families.
```
db_bench: ./include/rocksdb/slice.h:168: void rocksdb::PinnableSlice::PinSlice(const rocksdb::Slice&, rocksdb::Cleanable*): Assertion `!pinned_' failed.
```
This happens in `BlockBasedTable::Get()`.

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

Test Plan:
./db_bench --benchmarks=fillseq -num_column_families=3
./db_bench --benchmarks=readrandom -use_existing_db=1 -num_column_families=3

Reviewed By: ajkr

Differential Revision: D27587589

Pulled By: riversand963

fbshipit-source-id: 7379e7649ba40f046d6a4014c9ad629cb3f9a786
2021-04-06 11:31:17 -07:00
Adam Retter ffd3f493e3 Update ZStd. Fixes an issue with Make 3.82 (#8155)
Summary:
The previous version of ZStd doesn't build correctly with Make 3.82. Updating it resolves the issue.

jay-zhuang This also needs to be cherry-picked to:
1. 6.17.fb
2. 6.18.fb
3. 6.19.fb

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

Reviewed By: riversand963

Differential Revision: D27596460

Pulled By: ajkr

fbshipit-source-id: ac8492245e6273f54efcc1587346a797a91c9441
2021-04-06 11:05:10 -07:00
darionyaphet b2c48a570f Support cpu_write_nanos and cpu_read_nanos in IOStatsContext (#8149)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8149

Reviewed By: ajkr

Differential Revision: D27571017

Pulled By: riversand963

fbshipit-source-id: a73427e907a7cb899debf55d60a2ede726695277
2021-04-06 00:31:53 -07:00
David Carlier 88c8f7a090 stack trace freebsd update. using native api to get the process (#8144)
Summary:
full name.

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

Reviewed By: ajkr

Differential Revision: D27581146

Pulled By: riversand963

fbshipit-source-id: 7d4cbde02a07aa4676e35aeb60c3d6f1f492a3cd
2021-04-06 00:28:47 -07:00
Peter Dillinger 96205baa63 Likely fix flaky TableFileCorruptedBeforeBackup (#8151)
Summary:
Before corrupting a file in the DB and expecting corruption to
be detected, open DB read-only to ensure file is not made obsolete by
compaction. Also, to avoid obsolete files not yet deleted, only select
live files to corrupt.

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

Test Plan: watch CI

Reviewed By: akankshamahajan15

Differential Revision: D27568849

Pulled By: pdillinger

fbshipit-source-id: 39a69a2eafde0482b20a197949d24abe21952f27
2021-04-05 11:40:31 -07:00
Peter Dillinger bd7ddf58cb Make tests "parallel" and "passing ASC" by default (#8146)
Summary:
New tests should by default be expected to be parallelizeable
and passing with ASSERT_STATUS_CHECKED. Thus, I'm changing those two
lists to exclusions rather than inclusions.

For the set of exclusions, I only listed things that currently failed
for me when attempting not to exclude, or had some other documented
reason. This marks many more tests as "parallel," which will potentially
cause some failures from self-interference, but we can address those as
they are discovered.

Also changed CircleCI ASC test to be parallelized; the easy way to do
that is to exclude building tests that don't pass ASC, which is now a
small set.

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

Test Plan: Watch CI, etc.

Reviewed By: riversand963

Differential Revision: D27542782

Pulled By: pdillinger

fbshipit-source-id: bdd74bcd912a963ee33f3fc0d2cad2567dc7740f
2021-04-04 20:10:11 -07:00
Andrew Gallagher d0d2ab0b1a Use include_paths instead of raw -I in TARGETS (#8143)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8143

The latter assume the location of the compile root, which can break
if the build root changes.  Switch to the slightly more intelligent
`include_paths`, which should provide the same functionality, but do
with independent of include root.

Reviewed By: riversand963

Differential Revision: D27535869

fbshipit-source-id: 0129e47c0ce23e08528c9139114a591c14866fa8
2021-04-03 14:42:22 -07:00
Yanqin Jin dd3fbbbf95 Use separate db dir for different tests hoping to remove flakiness (#8147)
Summary:
DBWALTestWithParam relies on `SstFileManager` to have the expected behavior. However, if this test shares
db directories with other DBSSTTest, then the SstFileManager may see non-empty data, thus will change its
behavior to be different from expectation, introducing flakiness.

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

Test Plan: make check

Reviewed By: jay-zhuang

Differential Revision: D27553362

Pulled By: riversand963

fbshipit-source-id: a2d86343e8e2220bc553b6695ce87dd21a97ddec
2021-04-03 11:48:56 -07:00
Peter Dillinger 0fccc6225e Fix db_test2 parallelism (#8145)
Summary:
With thread/process-specific dirs. (Errors seen in FB infra.)

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

Test Plan: see in FB infra tests

Reviewed By: riversand963

Differential Revision: D27542355

Pulled By: pdillinger

fbshipit-source-id: b3c8e66f91a6a6b3a775f6fc0c3cf71e63c29ade
2021-04-02 13:38:04 -07:00
Akanksha Mahajan 689b13e639 Add request_id in IODebugContext. (#8045)
Summary:
Add request_id in IODebugContext which will be populated by
    underlying FileSystem for IOTracing purposes. Update IOTracer to trace
    request_id in the tracing records. Provided API
    IODebugContext::SetRequestId which will set the request_id and enable
    tracing for request_id. The API hides the implementation and underlying
    file system needs to call this API directly.

Update DB::StartIOTrace API and remove redundant Env* from the
    argument as its not used and DB already has Env that is passed down to
    IOTracer.

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

Test Plan: Update unit test.

Differential Revision: D26899871

Pulled By: akankshamahajan15

fbshipit-source-id: 56adef52ee5af0fb3060b607c3af1ec01635fa2b
2021-04-01 13:14:51 -07:00
rockeet 5025c7ec09 version_set_test.cc: remove a redundent obj copy (#7880)
Summary:
Remove redundant obj copy

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

Reviewed By: akankshamahajan15

Differential Revision: D26921119

Pulled By: riversand963

fbshipit-source-id: f227da688b067870a069e728a67799a8a95fee99
2021-04-01 11:28:54 -07:00
Zhichao Cao 17002365c1 Replace Status with IOStatus for block fetcher IO function (#8130)
Summary:
To propagate the IOStatus from file reads to RocksDB read logic, some of the existing status needs to be replaced by IOStatus.

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

Test Plan: make check

Reviewed By: anand1976

Differential Revision: D27440188

Pulled By: zhichao-cao

fbshipit-source-id: bbe7622c2106fe4e46871d60f7c26944e5030d78
2021-04-01 10:07:55 -07:00
Andrew Kryczka c43a37a922 Fix compression dictionary sampling with dedicated range tombstone SSTs (#8141)
Summary:
Return early in case there are zero data blocks when
`BlockBasedTableBuilder::EnterUnbuffered()` is called. This crash can
only be triggered by applying dictionary compression to SST files that
contain only range tombstones. It cannot be triggered by a low buffer
limit alone since we only consider entering unbuffered mode after
buffering a data block causing the limit to be breached, or `Finish()`ing the file. It also cannot
be triggered by a totally empty file because those go through
`Abandon()` rather than `Finish()` so unbuffered mode is never entered.

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

Test Plan: added a unit test that repro'd the "Floating point exception"

Reviewed By: riversand963

Differential Revision: D27495640

Pulled By: ajkr

fbshipit-source-id: a463cfba476919dc5c5c380800a75a86c31ffa23
2021-04-01 05:08:17 -07:00
darionyaphet a3a943bf63 Merge checks into one (#8138)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8138

Reviewed By: zhichao-cao

Differential Revision: D27475616

Pulled By: riversand963

fbshipit-source-id: d2815eed578a90c53d6a4e0dc4aaa232516eb4f8
2021-03-31 19:13:10 -07:00
Andrew Kryczka 1ba2b8a568 Add sample_for_compression results to table properties (#8139)
Summary:
Added `TableProperties::{fast,slow}_compression_estimated_data_size`.
These properties are present in block-based tables when
`ColumnFamilyOptions::sample_for_compression > 0` and the necessary
compression library is supported when the file is generated. They
contain estimates of what `TableProperties::data_size` would be if the
"fast"/"slow" compression library had been used instead. One
limitation is we do not record exactly which "fast" (ZSTD or Zlib)
or "slow" (LZ4 or Snappy) compression library produced the result.

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

Test Plan:
- new unit test
- ran `db_bench` with `sample_for_compression=1`; verified the `data_size` property matches the `{slow,fast}_compression_estimated_data_size` when the same compression type is used for the output file compression and the sampled compression

Reviewed By: riversand963

Differential Revision: D27454338

Pulled By: ajkr

fbshipit-source-id: 9529293de93ddac7f03b2e149d746e9f634abac4
2021-03-31 18:21:50 -07:00
Jay Zhuang a781b103da Fix getApproximateMemTableStats() return type (#8098)
Summary:
Which should return 2 long instead of an array.

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

Reviewed By: mrambacher

Differential Revision: D27308741

Pulled By: jay-zhuang

fbshipit-source-id: 44beea2bd28cf6779b048bebc98f2426fe95e25c
2021-03-31 09:46:47 -07:00
mrambacher 493a4e28d9 Pass PLATFORM_FLAGS in build_detect_platform (#8111)
Summary:
At least under MacOS, some things were excluded from the build (like Snappy) because the compilation flags were not passed in correctly.  This PR does a few things:
- Passes the EXTRA_CXX/LDFLAGS into build_detect_platform.  This means that if some tool (like TBB for example) is not installed in a standard place, it could still be detected by build_detect_platform.  In this case, the developer would invoke: "EXTRA_CXXFLAGS=<path to TBB include> EXTRA_LDFLAGS=<path to TBB library> make", and the build script would find the tools in the extra location.
- Changes the compilation tests to use PLATFORM_CXXFLAGS.  This change causes the EXTRA_FLAGS passed in to the script to be included in the compilation check.  Additionally, flags set by the script itself (like --std=c++11) will be used during the checks.

Validated that the make_platform.mk file generated on Linux does not change with this change.  On my MacOS machine, the SNAPPY libraries are now available (they were not before as they required --std=c++11 to build).

I also verified that I can build against TBB installed on my Mac by passing in the EXTRA CXX and LD FLAGS to the location in which TBB is installed.

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

Reviewed By: jay-zhuang

Differential Revision: D27353516

Pulled By: mrambacher

fbshipit-source-id: b6b378c96dbf678bab1479556dcbcb49c47e807d
2021-03-31 07:40:46 -07:00
Zhichao Cao 335c5a6be5 Fix error_handler_fs_test failure due to statistics (#8136)
Summary:
Fix error_handler_fs_test failure due to statistics, it will fails due to multi-thread running and resume is different.

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

Test Plan: make check

Reviewed By: akankshamahajan15

Differential Revision: D27448828

Pulled By: zhichao-cao

fbshipit-source-id: b94255c45e9e66e93334b5ca2e4e1bfcba23fc20
2021-03-30 21:44:44 -07:00
sherriiiliu e6534900bd Fix possible hang issue in ~DBImpl() when flush is scheduled in LOW pool (#8125)
Summary:
In DBImpl::CloseHelper, we wait for bg_compaction_scheduled_
and bg_flush_scheduled_ to drop to 0. Unschedule is called prior
to cancel any unscheduled flushes/compactions. It is assumed that
anything in the high priority is a flush, and anything in the low
priority pool is a compaction. This assumption, however, is broken when
the high-pri pool is full.
As a result, bg_compaction_scheduled_ can go < 0 and bg_flush_scheduled_
will remain > 0 and DB can be in hang state.
The fix is, we decrement the `bg_{flush,compaction,bottom_compaction}_scheduled_`
inside the `Unschedule{Flush,Compaction,BottomCompaction}Callback()`s. DB
`mutex_` will make the counts atomic in `Unschedule`.
Related discussion: https://github.com/facebook/rocksdb/issues/7928

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

Test Plan: Added new test case which hangs without the fix.

Reviewed By: jay-zhuang

Differential Revision: D27390043

Pulled By: ajkr

fbshipit-source-id: 78a367fba9a59ac5607ad24bd1c46dc16d5ec110
2021-03-30 18:35:20 -07:00
Jay Zhuang 9418403c4b Unittest uses unique test db name (#8124)
Summary:
thread_id is only unique within a process. If we run the same test-set with multiple processes, it could cause db path collision between 2 runs, error message will be like:
```
...
IO error: While lock file: /tmp/rocksdbtest-501//deletefile_test_8093137327721791717/LOCK: Resource temporarily unavailable
...
```
This is could be likely reproduced by:
```
gtest-parallel ./deletefile_test --gtest_filter=DeleteFileTest.BackgroundPurgeCFDropTest -r 1000 -w 1000
```

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

Reviewed By: ajkr

Differential Revision: D27435195

Pulled By: jay-zhuang

fbshipit-source-id: 850fc72cdb660edf93be9a1ca9327008c16dd720
2021-03-30 16:51:26 -07:00
Akanksha Mahajan f03606cd5c Vulnerability issue in kramdown dependency (#8131)
Summary:
GitHub has detected that a package defined in the
docs/Gemfile.lock file of the facebook/rocksdb repository contains a
security vulnerability.
This patch fixes it by upgrading the version of kramdown to 2.3.1

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

Reviewed By: jay-zhuang

Differential Revision: D27418776

Pulled By: akankshamahajan15

fbshipit-source-id: 0a4b0b85922b9958afcbc44560584701b1c6c82d
2021-03-30 10:31:27 -07:00
Peter Dillinger ec11c23caa Add thread safety to BackupEngine, explain more (#8115)
Summary:
BackupEngine previously had unclear but strict concurrency
requirements that the API user must follow for safe use. Now we make
that clear, by separating operations into "Read," "Append," and "Write"
operations, and specifying which combinations are safe across threads on
the same BackupEngine object (previously none; now all, using a
read-write lock), and which are safe across different BackupEngine
instances open on the same backup_dir.

The changes to backupable_db.h should be backward compatible. It is
mostly about eliminating copies of what should be the same function and
(unsurprisingly) useful documentation comments were often placed on
only one of the two copies. With the re-organization, we are also
grouping different categories of operations. In the future we might add
BackupEngineReadAppendOnly, but that didn't seem necessary.

To mark API Read operations 'const', I had to mark some implementation
functions 'const' and some fields mutable.

Functional changes:
* Added RWMutex locking around public API functions to implement thread
safety on a single object. To avoid future bugs, this is another
internal class layered on top (removing many "override" in
BackupEngineImpl). It would be possible to allow more concurrency
between operations, rather than mutual exclusion, but IMHO not worth the
work.
* Fixed a race between Open() (Initialize()) and CreateNewBackup() for
different objects on the same backup_dir, where Initialize() could
delete the temporary meta file created during CreateNewBackup().
(This was found by the new test.)

Also cleaned up a couple of "status checked" TODOs, and improved a
checksum mismatch error message to include involved files.

Potential follow-up work:
* CreateNewBackup has an API wart because it doesn't tell you the
BackupID it just created, which makes it of limited use in a multithreaded
setting.
* We could also consider a Refresh() function to catch up to
changes made from another BackupEngine object to the same dir.
* Use a lock file to prevent multiple writer BackupEngines, but this
won't work on remote filesystems not supporting lock files.

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

Test Plan:
new mini-stress test in backup unit tests, run with gcc,
clang, ASC, TSAN, and UBSAN, 100 iterations each.

Reviewed By: ajkr

Differential Revision: D27347589

Pulled By: pdillinger

fbshipit-source-id: 28d82ed2ac672e44085a739ddb19d297dad14b15
2021-03-29 22:41:51 -07:00
Imanol-Mikel Barba Sabariego 04191e1c5d Adding safer permissions to PosixFilesystem::NewLogger (#8106)
Summary:
We have observed rocksdb databases creating info log files with world-writeable permissions.

The reason why the file is created like so is because stdio streams opened with fopen calls use mode 0666, and while normally most systems have a umask of 022, in some occasions (for instance, while running daemons), you may find that the application is running with a less restrictive umask. The result is that when opening the DB, the LOG file would be created with world-writeable perms:

```
$ ls -lh db/
total 6.4M
-rw-r--r-- 1 ibarba users  115 Mar 24 17:41 000004.log
-rw-r--r-- 1 ibarba users   16 Mar 24 17:41 CURRENT
-rw-r--r-- 1 ibarba users   37 Mar 24 17:41 IDENTITY
-rw-r--r-- 1 ibarba users    0 Mar 24 17:41 LOCK
-rw-rw-r-- 1 ibarba users 114K Mar 24 17:41 LOG
-rw-r--r-- 1 ibarba users  514 Mar 24 17:41 MANIFEST-000003
-rw-r--r-- 1 ibarba users  31K Mar 24 17:41 OPTIONS-000018
-rw-r--r-- 1 ibarba users  31K Mar 24 17:41 OPTIONS-000020
```

This diff replaces the fopen call with a regular open() call restricting mode, and then using fdopen to associate an stdio stream with that file descriptor. Resulting in the following files being created:

```
-rw-r--r-- 1 ibarba users   58 Mar 24 18:16 000004.log
-rw-r--r-- 1 ibarba users   16 Mar 24 18:16 CURRENT
-rw-r--r-- 1 ibarba users   37 Mar 24 18:16 IDENTITY
-rw-r--r-- 1 ibarba users    0 Mar 24 18:16 LOCK
-rw-r--r-- 1 ibarba users 111K Mar 24 18:16 LOG
-rw-r--r-- 1 ibarba users  514 Mar 24 18:16 MANIFEST-000003
-rw-r--r-- 1 ibarba users  31K Mar 24 18:16 OPTIONS-000018
-rw-r--r-- 1 ibarba users  31K Mar 24 18:16 OPTIONS-000020
```

With the correct permissions

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

Reviewed By: akankshamahajan15

Differential Revision: D27415377

Pulled By: mrambacher

fbshipit-source-id: 97ac6c215700a7ea306f4a1fdf9fcf64a3cbb202
2021-03-29 20:47:21 -07:00
Jay Zhuang a037bb35e9 Compaction should not move data to up level (#8116)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8116

Reviewed By: ajkr, mrambacher

Differential Revision: D27353828

Pulled By: jay-zhuang

fbshipit-source-id: 42703fb01b04d92cc097d7979e64798448852e88
2021-03-29 17:10:42 -07:00
Adam Retter 24b7ebee80 range_tree requires GNU libc on ppc64 (#8070)
Summary:
If the platform is ppc64 and the libc is not GNU libc, then we exclude the range_tree from compilation.

See https://jira.percona.com/browse/PS-7559

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

Reviewed By: jay-zhuang

Differential Revision: D27246004

Pulled By: mrambacher

fbshipit-source-id: 59d8433242ce7ce608988341becb4f83312445f5
2021-03-29 16:32:08 -07:00
kshair 25ae380784 Fix comment spelling (#7960)
Summary:
terated -> treated

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

Reviewed By: ajkr

Differential Revision: D26677005

Pulled By: zhichao-cao

fbshipit-source-id: 6221305afb263aa60f674a4113aa30cb8f3914e6
2021-03-29 10:37:24 -07:00
mrambacher 1be3867689 Fix check in db_bench for num shard bits to match check in LRUCache (#8110)
Summary:
The check in db_bench for table_cache_numshardbits was 0 < bits <= 20, whereas the check in LRUCache was 0 < bits < 20.  Changed the two values to match to avoid a crash in db_bench on a null cache.

Fixes https://github.com/facebook/rocksdb/issues/7393

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

Reviewed By: zhichao-cao

Differential Revision: D27353522

Pulled By: mrambacher

fbshipit-source-id: a414bd23b5bde1f071146b34cfca5e35c02de869
2021-03-29 10:34:54 -07:00
yaphet 70e80c91b6 fix typo (#8118)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8118

Reviewed By: ajkr

Differential Revision: D27367488

Pulled By: zhichao-cao

fbshipit-source-id: 6ed598c74ab9232f2e56326b3a30476d473699d7
2021-03-29 10:32:10 -07:00
mrambacher 524b10bd6e Fix spelling in comments in include/rocksdb/ (#8120)
Summary:
Ran a spell check over the comments in the include/rocksdb directory and fixed any mis-spellings.

There are still some variable names that are spelled incorrectly (like SizeApproximationOptions::include_memtabtles, SstFileMetaData::oldest_ancester_time) that were not fixed, as those would break compilation.

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

Reviewed By: zhichao-cao

Differential Revision: D27366034

Pulled By: mrambacher

fbshipit-source-id: 6a3f3674890bb6acc751e9c5887a8fbb6adca5df
2021-03-29 05:05:06 -07:00
Yanqin Jin ae7a795686 Disable partitioned filters in ts stress test (#8127)
Summary:
Currently, partitioned filter does not support user-defined timestamp. Disable it for now in ts stress test so that
the contrun jobs can proceed.

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

Test Plan: make crash_test_with_ts

Reviewed By: ajkr

Differential Revision: D27388488

Pulled By: riversand963

fbshipit-source-id: 5ccff18121cb537bd82f2ac072cd25efb625c666
2021-03-29 00:40:52 -07:00
mrambacher 5841bbe36c Fix make tags to not rebuild all the object files (#8097)
Summary:
Because build_version.cc is dependent on the library objects (to force a re-generation of it), the library objects would be built in order to satisfy this rule.  Because there is a build_version.d file, it would need generated and included.

Change the ALL_DEPS/FILES to not include build_version.cc (meaning no .d file for it, which is okay since it is generated).  Also changed the rule on whether or not to generate DEP files to skip tags.

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

Reviewed By: ajkr

Differential Revision: D27299815

Pulled By: mrambacher

fbshipit-source-id: 1efbe8a56d062f57ae13b6c2944ad3faf775087e
2021-03-28 21:11:18 -07:00