Commit Graph

10127 Commits

Author SHA1 Message Date
Baptiste Lemaire
0a1aed4e71 Fix double slashes in user-provided db path. (#8439)
Summary:
At the moment, the following command : "`./ --db=mypath/ dump_file_files`" returns a series of erronous names with double slashes, ie: "`mypath//000xxx.sst`", including manifest file names with double slashes "`mypath//MANIFEST-00XXX`", whereas "`./ --db=mypath dump_file_files`" correctly returns "`mypath/000xxx.sst`" and "`mypath/MANIFEST-00XXX`".

This (very short) PR simply checks if there is a need to add or remove any '`/`' character when the `db_path` and `manifest_filename`/sst `filenames` are concatenated.

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

Reviewed By: akankshamahajan15

Differential Revision: D29301349

Pulled By: bjlemaire

fbshipit-source-id: 3e9e58f9749d278b654ae838fcee13ad698705a8
2021-06-22 11:46:25 -07:00
Jay Zhuang
f89423a57a Revert "Revert "Snapshot release triggered compaction without multiple tombstones (#8357)" (#8410)" (#8438)
Summary:
This reverts commit 25be1ed66a.

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

Test Plan: Run the impacted mysql test 40 times

Reviewed By: ajkr

Differential Revision: D29286247

Pulled By: jay-zhuang

fbshipit-source-id: d3bd056971a19a8b012d5d0295fa045c012b3c04
2021-06-22 11:10:03 -07:00
Andrew Kryczka
b511b042f4 Fixup HISTORY.md for 6.22 release (#8441)
Summary:
`git diff origin/6.21.fb origin/6.22.fb -- HISTORY.md` looked odd.
This PR fixes it up by moving items from 6.21.0 to 6.22.0 that were
never in any 6.21 release. Also mentioned the background stat collection
fix under 6.22 (previously it was mentioned under 6.21 patch releases
only).

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

Reviewed By: jay-zhuang

Differential Revision: D29304812

Pulled By: ajkr

fbshipit-source-id: 2a928a9518a1d6615321d5c2d1e22b17cbb59093
2021-06-22 10:30:10 -07:00
Levi Tamasi
cbb3b25915 Print blob file checksums as hex (#8437)
Summary:
Currently, blob file checksums are incorrectly dumped as raw bytes
in the `ldb manifest_dump` output (i.e. they are not printed as hex).
The patch fixes this and also updates some test cases to reflect that
the checksum value field in `BlobFileAddition` and `SharedBlobFileMetaData`
contains the raw checksum and not a hex string.

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

Test Plan:
`make check`
Tested using `ldb manifest_dump`

Reviewed By: akankshamahajan15

Differential Revision: D29284170

Pulled By: ltamasi

fbshipit-source-id: d11cfb3435b14cd73c8a3d3eb14fa0f9fa1d2228
2021-06-22 09:49:44 -07:00
Jay Zhuang
54d73d6429 Fix DeleteFilesInRange may cause inconsistent compaction error (#8434)
Summary:
`DeleteFilesInRange()` marks deleting files to `being_compacted`
before deleting, which may cause ongoing compactions report corruption
exception or ASSERT for debug build.

Adding the missing `ComputeCompactionScore()` when `being_compacted` is set.

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

Test Plan: Unittest

Reviewed By: ajkr

Differential Revision: D29276127

Pulled By: jay-zhuang

fbshipit-source-id: f5b223e3c1fc6d821e100e3f3442bc70c1d50cf7
2021-06-22 09:17:37 -07:00
Levi Tamasi
065bea1587 Add a class for measuring the amount of garbage generated during compaction (#8426)
Summary:
This is part of an alternative approach to https://github.com/facebook/rocksdb/issues/8316.
Unlike that approach, this one relies on key-values getting processed one by one
during compaction, and does not involve persistence.

Specifically, the patch adds a class `BlobGarbageMeter` that can track the number
and total size of blobs in a (sub)compaction's input and output on a per-blob file
basis. This information can then be used to compute the amount of additional
garbage generated by the compaction for any given blob file by subtracting the
"outflow" from the "inflow."

Note: this patch only adds `BlobGarbageMeter` and associated unit tests. I plan to
hook up this class to the input and output of `CompactionIterator` in a subsequent PR.

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

Test Plan: `make check`

Reviewed By: jay-zhuang

Differential Revision: D29242250

Pulled By: ltamasi

fbshipit-source-id: 597e50ad556540e413a50e804ba15bc044d809bb
2021-06-21 22:25:30 -07:00
anand76
a50da404be Fix a tsan warning due to reading flags in LRUHandle without holding a mutex (#8433)
Summary:
Tsan complains due to a perceived race condition in accessing LRUHandle flags. One thread calls ```LRUHandle::SetHit()``` from ```LRUCacheShard::Lookup()```, while another thread calls ```LRUHandle::IsPending()``` from ```LRUCacheShard::IsReady()```. The latter call is from ```MultiGet```. It doesn't actually have to call ```IsReady``` since a null value indicates the cache handle is not ready, so its sufficient to check for a null value.

Also modify ```IsReady``` to acquire the LRU shard mutex.

Tests:
1. make check
2. Run tsan_crash

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

Reviewed By: zhichao-cao

Differential Revision: D29278030

Pulled By: anand1976

fbshipit-source-id: 0c9fed56d12eda853e72dadebe75038361bd257f
2021-06-21 21:23:56 -07:00
lorinlee
e9b627fd56 Fix comments in Status (#8429)
Summary:
Signed-off-by: lorinlee <lorinlee1996@gmail.com>

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

Reviewed By: zhichao-cao

Differential Revision: D29256538

Pulled By: ajkr

fbshipit-source-id: fa483861148d82119f4c8775d0cf5f6c71b06215
2021-06-21 13:41:08 -07:00
Andrew Kryczka
6f33901899 Skip c_test and env_test when ASSERT_STATUS_CHECKED=1 (#8430)
Summary:
- `c_test` fails because `rocksdb_compact_range()` swallows a `Status`.
- `env_test` fails because `ReadRequest`s to `MultiRead()` do not have their `Status`es checked.

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

Test Plan: `ASSERT_STATUS_CHECKED=1 make -j48 check`

Reviewed By: jay-zhuang

Differential Revision: D29257473

Pulled By: ajkr

fbshipit-source-id: e02127f971703744be7de85f0a028e4664c79577
2021-06-21 13:15:31 -07:00
Andrew Kryczka
7ed69fbe72 Update HISTORY.md and version.h 6.22 release (#8427)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8427

Reviewed By: zhichao-cao

Differential Revision: D29246916

Pulled By: ajkr

fbshipit-source-id: ccd44ca1a6dd5101dc37f19b8e1fe6c0e3883e0a
2021-06-19 12:12:46 -07:00
mwish
19a89267ca typo: fix typo in db/write_thread's state (#8423)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8423

Reviewed By: mrambacher

Differential Revision: D29232587

Pulled By: jay-zhuang

fbshipit-source-id: 04d4937cf0605cbf341a920d1305369a7b8f0574
2021-06-18 17:14:51 -07:00
Zhichao Cao
82a70e1470 Trace MultiGet Keys and CF_IDs to the trace file (#8421)
Summary:
Tracing the MultiGet information including timestamp, keys, and CF_IDs to the trace file for analyzing and replay.

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

Test Plan: make check, add test to trace_analyzer_test

Reviewed By: anand1976

Differential Revision: D29221195

Pulled By: zhichao-cao

fbshipit-source-id: 30c677d6c39ab31ef4bbdf7e0d1fa1fd79f295ff
2021-06-18 15:04:05 -07:00
Akanksha Mahajan
d96b0127a0 Change Windows image back to stable (#8424)
Summary:
Since windows timeout issue has been fixed. Change the image back to
stable.

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

Test Plan: Check CircleCI jobs

Reviewed By: zhichao-cao

Differential Revision: D29235219

Pulled By: akankshamahajan15

fbshipit-source-id: 2c111f96e216dac4ae3d7ec3b4cdd8e459575d37
2021-06-18 12:32:01 -07:00
anand76
8ea0a2c1bd Parallelize secondary cache lookup in MultiGet (#8405)
Summary:
Implement the ```WaitAll()``` interface in ```LRUCache``` to allow callers to issue multiple lookups in parallel and wait for all of them to complete. Modify ```MultiGet``` to use this to parallelize the secondary cache lookups in order to reduce the overall latency. A call to ```cache->Lookup()``` returns a handle that has an incomplete value (nullptr), and the caller can call ```cache->IsReady()``` to check whether the lookup is complete, and pass a vector of handles to ```WaitAll``` to wait for completion. If any of the lookups fail, ```MultiGet``` will read the block from the SST file.

Another change in this PR is to rename ```SecondaryCacheHandle``` to ```SecondaryCacheResultHandle``` as it more accurately describes the return result of the secondary cache lookup, which is more like a future.

Tests:
1. Add unit tests in lru_cache_test
2. Benchmark results with no secondary cache configured
Master -
```
readrandom   :      41.175 micros/op 388562 ops/sec;  106.7 MB/s (7277999 of 7277999 found)
readrandom   :      41.217 micros/op 388160 ops/sec;  106.6 MB/s (7274999 of 7274999 found)
multireadrandom :      10.309 micros/op 1552082 ops/sec; (28908992 of 28908992 found)
multireadrandom :      10.321 micros/op 1550218 ops/sec; (29081984 of 29081984 found)
```

This PR -
```
readrandom   :      41.158 micros/op 388723 ops/sec;  106.8 MB/s (7290999 of 7290999 found)
readrandom   :      41.185 micros/op 388463 ops/sec;  106.7 MB/s (7287999 of 7287999 found)
multireadrandom :      10.277 micros/op 1556801 ops/sec; (29346944 of 29346944 found)
multireadrandom :      10.253 micros/op 1560539 ops/sec; (29274944 of 29274944 found)
```

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

Reviewed By: zhichao-cao

Differential Revision: D29190509

Pulled By: anand1976

fbshipit-source-id: 6f8eff6246712af8a297cfe22ea0d1c3b2a01bb0
2021-06-18 09:35:59 -07:00
Baptiste Lemaire
e817bc9628 Added memtable garbage statistics (#8411)
Summary:
**Summary**:
2 new statistics counters are added to RocksDB: `MEMTABLE_PAYLOAD_BYTES_AT_FLUSH` and `MEMTABLE_GARBAGE_BYTES_AT_FLUSH`. The former tracks how many raw bytes of useful data are present on the memtable at flush time, whereas the latter is tracks how many of these raw bytes are considered garbage, meaning that they ended up not being imported on the SSTables resulting from the flush operations.

**Unit test**: run `make db_flush_test -j$(nproc); ./db_flush_test` to run the unit test.
This executable includes 3 tests, that test support and correct stat calculations for workloads with inserts, deletes, and DeleteRanges. The parameters are set such that the workloads are performed on a single memtable, and a single SSTable is created as a result of the flush operation. The flush operation is manually called in the test file. The tests verify that the values of these 2 statistics counters introduced in this PR  can be exactly predicted, showing that we have a full understanding of the underlying operations.

**Performance testing**:
`./db_bench -statistics -benchmarks=fillrandom -num=10000000` repeated 10 times.
Timing done using "date" function in a bash script.
_Results_:
Original Rocksdb fork: mean 66.6 sec, std 1.18 sec.
This feature branch: mean 67.4 sec, std 1.35 sec.

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

Reviewed By: akankshamahajan15

Differential Revision: D29150629

Pulled By: bjlemaire

fbshipit-source-id: 7b3c2e86d50c6aa34fa50fd134282eacb543a5b1
2021-06-18 04:57:27 -07:00
Akanksha Mahajan
5ba1b6e549 Cache warming data blocks during flush (#8242)
Summary:
This PR prepopulates warm/hot data blocks which are already in memory
into block cache at the time of flush. On a flush, the data block that is
in memory (in memtables) get flushed to the device. If using Direct IO,
additional IO is incurred to read this data back into memory again, which
is avoided by enabling newly added option.

 Right now, this is enabled only for flush for data blocks. We plan to
expand this option to cover compactions in the future and for other types
 of blocks.

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

Test Plan: Add new unit test

Reviewed By: anand1976

Differential Revision: D28521703

Pulled By: akankshamahajan15

fbshipit-source-id: 7219d6958821cedce689a219c3963a6f1a9d5f05
2021-06-17 21:56:47 -07:00
聂佩轩
d53f7ff69a Add DeteleRange support for DBWithTTL (#8384)
Summary:
This commit is for enabling `DBWithTTL` to use `DeteleRange` which it cannot before.
As (int32_t)Timestamp is suffixed to values in `DBWithTTL`, there is no reason that it
cannot use the common used api. I added `DeleteRangeCF` in `DBWithTTLImpl::Write`
so that we can use `DeteleRange` normally. When we run code like
`dbWithTtl->DeleteRange(start, end)`, it executes`WriteBatchInternal::DeleteRange`
internally. Intended to fix https://github.com/facebook/rocksdb/issues/7218

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

Test Plan: added corresponded testing logic to existing unit test

Reviewed By: jay-zhuang

Differential Revision: D29176734

fbshipit-source-id: 6874ed979fc08e1d138149d03653e43a75f0e0e6
2021-06-17 16:00:50 -07:00
Peter Dillinger
865a25101d Mark Ribbon filter and optimize_filters_for_memory as production (#8408)
Summary:
Marked the Ribbon filter and optimize_filters_for_memory features
as production-ready, each enabling memory savings for Bloom-like filters.
Use `NewRibbonFilterPolicy` in place of `NewBloomFilterPolicy` to use
Ribbon filters instead of Bloom, or `ribbonfilter` in place of
`bloomfilter` in configuration string.

Some small refactoring in db_stress.

Removed/refactored unused code in db_bench, in part preparing for future
default possibly being different from "disabled."

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

Test Plan:
Lots of prior automated, ad-hoc, and "real world" testing.
Updated tests for new API names. Quick db_bench test:

bloom fillrandom
77730 ops/sec
rocksdb.block.cache.filter.bytes.insert COUNT : 89929384

ribbon fillrandom
71492 ops/sec
rocksdb.block.cache.filter.bytes.insert COUNT : 64531384

Reviewed By: mrambacher

Differential Revision: D29140805

Pulled By: pdillinger

fbshipit-source-id: d742c922722421678f95ad85eeb0aaebc9f5e49a
2021-06-17 12:29:16 -07:00
anand76
575ea26ec9 Don't log a warning if file system doesn't support ReopenWritableFile() (#8414)
Summary:
RocksDB logs a warning if WAL truncation on DB open fails. Its possible that on some file systems, truncation is not required and they would return ```Status::NotSupported()``` for ```ReopenWritableFile```. Don't log a warning in such cases.

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

Reviewed By: akankshamahajan15

Differential Revision: D29181738

Pulled By: anand1976

fbshipit-source-id: 6e01e9117e1e4c1d67daa4dcee7fa59d06e057a7
2021-06-17 12:05:40 -07:00
Sidi Mohamed EL AATIFI
298edae941 Fix a typo in Javadoc (#8394)
Summary:
iterateLowerBound Slice representing the lower bound

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

Reviewed By: ajkr

Differential Revision: D29085721

Pulled By: jay-zhuang

fbshipit-source-id: a154375879395c48e9bd3794d296e70316894056
2021-06-17 12:02:57 -07:00
mrambacher
d81c2d1e3d Add Inner method and support for wrapped Customizable (#8373)
Summary:
Inner method added for classes to override to return has-a relationship.  CheckedCast expanded to use Inner to return has-a relationship.

Future Customizable classes (Env, FileSystem, Statistics, etc) will use this feature

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

Reviewed By: pdillinger

Differential Revision: D29176369

Pulled By: mrambacher

fbshipit-source-id: cfb6d7702fa365ca4e40c4a50a19e3a534e5ac43
2021-06-17 08:39:36 -07:00
Zhichao Cao
ecccc63179 Implementation of Crc32c combine function (#8305)
Summary:
Implement a function to generate the crc32c of two combined strings. Suppose we have the string 1 (s1) with crc32c checksum crc32c_1 and string 2 (s2) with crc32c checksum crc32c_2, the new string is s1+s2 and its checksum is crc32c_new=Crc32cCombine(crc32c_1, crc32c_2, s2.size).

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

Test Plan: make check, added new testing case

Reviewed By: pdillinger

Differential Revision: D28651665

Pulled By: zhichao-cao

fbshipit-source-id: c84116108388f11a81f6a217b49f99c70d4ffacf
2021-06-16 18:30:34 -07:00
mrambacher
d5bd0039b9 Rename ImmutableOptions variables (#8409)
Summary:
This is the next part of the ImmutableOptions cleanup.  After changing the use of ImmutableCFOptions to ImmutableOptions, there were places in the code that had did something like "ImmutableOptions* immutable_cf_options", where "cf" referred to the "old" type.

This change simply renames the variables to match the current type.  No new functionality is introduced.

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

Reviewed By: pdillinger

Differential Revision: D29166248

Pulled By: mrambacher

fbshipit-source-id: 96de97f8e743f5c5160f02246e3ed8269556dc6f
2021-06-16 16:51:38 -07:00
Andrew Kryczka
25be1ed66a Revert "Snapshot release triggered compaction without multiple tombstones (#8357)" (#8410)
Summary:
This reverts commit 9167ece586.

It was found to reliably trip a compaction picking conflict assertion in a MyRocks unit test. We don't understand why yet so reverting in the meantime.

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

Test Plan: `make check -j48`

Reviewed By: jay-zhuang

Differential Revision: D29150300

Pulled By: ajkr

fbshipit-source-id: 2de8664f355d6da015e84e5fec2e3f90f49741c8
2021-06-15 18:15:15 -07:00
Peter Dillinger
32a4d59477 Remove TSAN crash test opt-in to platform007 (#8406)
Summary:
Facebook internal test failure

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

Test Plan: COMPILE_WITH_TSAN=1 make crash_test_with_txn

Reviewed By: akankshamahajan15

Differential Revision: D29124055

Pulled By: pdillinger

fbshipit-source-id: f0f15d6a71435043b19f2170e7500823de1cc43a
2021-06-15 08:17:01 -07:00
mrambacher
b788e3f497 Increase range for GeneralTableTest.ApproximateOffsetOfCompressed (#8387)
Summary:
Newer versions of Snappy (1.1 patch 8) were failing this test because the offsets were outside of the expected range.

In some experiments:
- On a RH machine with 1.1.0, the offset of "k04" and "xyy" were 3331 and 6665.
- On an Ubuntu machine with 1.1.8, the same keys were at 3501 and 7004.
- On a Mac with 1.1.8, the offsets were 3499 and 7001.

AFAICT, the test environments are either using an older version of Snappy or no Snappy at all.

This change increases the range to allow the tests to pass.

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

Reviewed By: pdillinger

Differential Revision: D29064475

Pulled By: mrambacher

fbshipit-source-id: fac01927576765b8aff9f57e08a63a2ae210855f
2021-06-15 03:46:30 -07:00
mrambacher
281ac9c89e Add CreateFrom methods to Env/FileSystem (#8174)
Summary:
- Added CreateFromString method to Env and FilesSystem to replace LoadEnv/Load.  This method/signature is a precursor to making these classes extend Customizable.

- Added CreateFromSystem to Env.  This method standardizes creating an Env from the environment variables.  Previously, some places would check TEST_ENV_URI and others would also check TEST_FS_URI.  Now the code is more command/standardized.

- Added CreateFromFlags to Env.  These method allows Env to be create from string options (such as GFLAGS options) in a more standard way.

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

Reviewed By: zhichao-cao

Differential Revision: D28999603

Pulled By: mrambacher

fbshipit-source-id: 88e6911e7e91f908458a7fe10a20e93ecbc275fb
2021-06-15 03:43:48 -07:00
Jay Zhuang
99908db998 Increase cmake test DISCOVERY_TIMEOUT from 5 to 120 seconds (#8403)
Summary:
cmake test discovery may timeout especially on Windows
platform. Increase it from default 5 seconds to 120 seconds.

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

Test Plan: Run Windows build 10 times without issue

Reviewed By: akankshamahajan15

Differential Revision: D29117455

Pulled By: jay-zhuang

fbshipit-source-id: 74f71833432f016776a59e070b0f4e146968f81b
2021-06-14 16:31:46 -07:00
Peter Dillinger
416dc7bed7 Clarify some DB::Open,OpenForReadOnly semantics (#8379)
Summary:
Longstanding tech debt

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

Test Plan: Better than not having an API contract

Reviewed By: jay-zhuang

Differential Revision: D29011131

Pulled By: pdillinger

fbshipit-source-id: 2c4796177733651954024fc17875f8642ca08d09
2021-06-14 16:09:21 -07:00
Peter Dillinger
3981430f54 Fix runtime linkage with libasan in Facebook platform009 (#8402)
Summary:
Was seeing

    ./cache_test: error while loading shared libraries: libasan.so.5: cannot open shared object file: No such file or directory

etc. using COMPILE_WITH_ASAN=1 without USE_CLANG=1

Now including compiler libs in runtime ld path.

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

Test Plan: reproduced with local builds

Reviewed By: akankshamahajan15

Differential Revision: D29107729

Pulled By: pdillinger

fbshipit-source-id: 13805b87b846b39522c9dd6a231ca245c58f1c71
2021-06-14 12:16:11 -07:00
Hui Xiao
dcddc1065e Make CompactionService derived from Customizable (#8395)
Summary:
(1)Make CompactionService derived from Customizable by defining two extra functions that are needed, as described in customizable.h comment section
(2)Revise the MyTestCompactionService class in compaction_service_test.cc to satisfy the class inheritance requirement
(3)Specify namespace of ToString() in compaction_service_test.cc to avoid function collision with CompactionService's ancestor classes

Test did:
make -j24 compaction_service_test
./compaction_service_test

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

Reviewed By: jay-zhuang

Differential Revision: D29076068

Pulled By: hx235

fbshipit-source-id: c130100fa466939b3137e917f5fdc4b2ae8e37d4
2021-06-14 11:41:57 -07:00
Akanksha Mahajan
9047fd7766 Fix Windows build failure (#8400)
Summary:
Fix window build failure by reverting to previous tag as suggested by CircleCI.

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

Test Plan: Watch CircleCI builds for a day or two for failure

Reviewed By: jay-zhuang

Differential Revision: D29104458

Pulled By: akankshamahajan15

fbshipit-source-id: 5df03092e4b0c221ee12daad7d1fdf8d35eb1082
2021-06-14 09:35:32 -07:00
Peter Dillinger
79407a8a61 Fix^2 use of binutils in Facebook platform009 (#8399) (#8401)
Summary:
Internal builds still failing, this time with ld

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

Test Plan: Like https://github.com/facebook/rocksdb/issues/8399 but letting build run to completion

Reviewed By: bjlemaire

Differential Revision: D29103512

Pulled By: pdillinger

fbshipit-source-id: 0fcad2c63518cf2b721e749881da40b90f5d3133
2021-06-14 08:45:21 -07:00
Peter Dillinger
d5a46c40e5 Pin CacheEntryStatsCollector to fix performance bug (#8385)
Summary:
If the block Cache is full with strict_capacity_limit=false,
then our CacheEntryStatsCollector could be immediately evicted on
release, so iterating through column families with shared block cache
could trigger re-scan for each CF. This change fixes that problem by
pinning the CacheEntryStatsCollector from InternalStats so that it's not
evicted.

I had originally thought that this object could participate in LRU like
everything else, but even though a re-load+re-scan only touches memory,
it can be orders of magnitude more expensive than other cache misses.
One service in Facebook has scans that take ~20s over 100GB block cache
that is mostly 4KB entries. (The up-side of this bug and https://github.com/facebook/rocksdb/issues/8369 is that
we had a natural experiment on the effect on some service metrics even
with block cache scans running continuously in the background--a kind
of worst case scenario. Metrics like latency were not affected enough
to trigger warnings.)

Other smaller fixes:

20s is already a sizable portion of 600s stats dump period, or 180s
default max age to force re-scan, so added logic to ensure that (for
each block cache) we don't spend more than 0.2% of our background thread
time scanning it. Nevertheless, "foreground" requests for cache entry
stats (calls to `db->GetMapProperty(DB::Properties::kBlockCacheEntryStats)`)
are permitted to consume more CPU.

Renamed field to cache_entry_stats_ to match code style.

This change is intended for patching in 6.21 release.

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

Test Plan:
unit test expanded to cover new logic (detect regression),
some manual testing with db_bench

Reviewed By: ajkr

Differential Revision: D29042759

Pulled By: pdillinger

fbshipit-source-id: 236faa902397f50038c618f50fbc8cf3f277308c
2021-06-14 08:15:11 -07:00
Jay Zhuang
d60ae5b1c7 Fix flaky ManualCompactionMax test (#8396)
Summary:
Recalculate the total size after generate new sst files.
New generated files might have different size as the previous time which
could cause the test failed.

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

Test Plan:
```
gtest-parallel ./db_compaction_test
--gtest_filter=DBCompactionTest.ManualCompactionMax -r 1000 -w 100
```

Reviewed By: akankshamahajan15

Differential Revision: D29083299

Pulled By: jay-zhuang

fbshipit-source-id: 49d4bd619cefc0f9a1f452f8759ff4c2ba1b6fdb
2021-06-14 08:11:40 -07:00
Peter Dillinger
0d0aa578ff Fix use of binutils in Facebook platform009 (#8399)
Summary:
Internal builds failing

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

Test Plan:
I can reproduce a failure by putting a bad version of `as` in
my PATH. This indicates that before this change, the custom compiler is
falsely relying on host `as`. This change fixes that, ignoring the bad
`as` on PATH.

Reviewed By: akankshamahajan15

Differential Revision: D29094159

Pulled By: pdillinger

fbshipit-source-id: c432e90404ea4d39d885a685eebbb08be9eda1c8
2021-06-13 23:33:31 -07:00
Levi Tamasi
146263887f Disable subcompactions for user-defined timestamps (#8393)
Summary:
The subcompaction boundary picking logic does not currently guarantee
that all user keys that differ only by timestamp get processed by the same
subcompaction. This can cause issues with the `CompactionIterator` state
machine: for instance, one subcompaction that processes a subset of such KVs
might drop a tombstone based on the KVs it sees, while in reality the
tombstone might not have been eligible to be optimized out.
(See also https://github.com/facebook/rocksdb/issues/6645, which adjusted the way compaction inputs are picked for the
same reason.)

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

Test Plan: Ran `make check` and the crash test script with timestamps enabled.

Reviewed By: jay-zhuang

Differential Revision: D29071635

Pulled By: ltamasi

fbshipit-source-id: f6c72442122b4e581871e096fabe3876a9e8a5a6
2021-06-12 12:09:25 -07:00
Peter Dillinger
b3dbeadc34 Fix double-dumping CF stats to log (#8380)
Summary:
DBImpl::DumpStats is supposed to do this:
Dump DB stats to LOG
For each CF, dump CFStatsNoFileHistogram to LOG
For each CF, dump CFFileHistogram to LOG

Instead, due to a longstanding bug from 2017 (https://github.com/facebook/rocksdb/issues/2126), it would dump
CFStats, which includes both CFStatsNoFileHistogram and CFFileHistogram,
in both loops, resulting in near-duplicate output.

This fixes the bug.

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

Test Plan: Manual inspection of LOG after db_bench

Reviewed By: jay-zhuang

Differential Revision: D29017535

Pulled By: pdillinger

fbshipit-source-id: 3010604c4a629a80347f129cd746ce9b0d0cbda6
2021-06-11 17:06:09 -07:00
Zhichao Cao
58162835d1 All the NoSpace() errors will be handled by regular SetBGError and RecoverFromNoSpace() (#8376)
Summary:
In the current logic, any IO Error with retryable flag == true will be handled by the special logic and in most cases, StartRecoverFromRetryableBGIOError will be called to do the auto resume. If the NoSpace error with retryable flag is set during WAL write, it is mapped as a hard error, which will trigger the auto recovery. During the recover process, if write continues and append to the WAL, the write process sees that bg_error is set to HardError and it calls WriteStatusCheck(), which calls SetBGError() with Status (not IOStatus). This will redirect to the regular SetBGError interface, in which recovery_error_ will be set to the corresponding error. With the recovery_error_ set, the auto resume thread created in StartRecoverFromRetryableBGIOError will keep failing as long as user keeps trying to write.

To fix this issue. All the NoSpace error (no matter retryable flag is set or not) will be redirect to the regular SetBGError, and RecoverFromNoSpace() will do the recovery job which calls SstFileManager::StartErrorRecovery().

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

Test Plan: make check and added the new testing case

Reviewed By: anand1976

Differential Revision: D29071828

Pulled By: zhichao-cao

fbshipit-source-id: 7171d7e14cc4620fdab49b7eff7a2fe9a89942c2
2021-06-11 14:48:28 -07:00
Peter Dillinger
a42a342a7a Make platform009 default for FB developers (#8389)
Summary:
platform007 being phased out and sometimes broken

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

Test Plan: `make V=1` to see which compiler is being used

Reviewed By: jay-zhuang

Differential Revision: D29067183

Pulled By: pdillinger

fbshipit-source-id: d1b07267cbc55baa9395f2f4fe3967cc6dad52f7
2021-06-11 11:37:05 -07:00
mrambacher
6ad0810393 Make Comparator into a Customizable Object (#8336)
Summary:
Makes the Comparator class into a Customizable object.  Added/Updated the CreateFromString method to create Comparators.  Added test for using the ObjectRegistry to create one.

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

Reviewed By: jay-zhuang

Differential Revision: D28999612

Pulled By: mrambacher

fbshipit-source-id: bff2cb2814eeb9fef6a00fddc61d6e34b6fbcf2e
2021-06-11 06:22:59 -07:00
Akanksha Mahajan
3897ce3125 Support for Merge in Integrated BlobDB with base values (#8292)
Summary:
This PR add support for Merge operation in Integrated BlobDB with base values(i.e DB::Put). Merged values can be retrieved through  DB::Get, DB::MultiGet, DB::GetMergeOperands and Iterator operation.

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

Test Plan: Add new unit tests

Reviewed By: ltamasi

Differential Revision: D28415896

Pulled By: akankshamahajan15

fbshipit-source-id: e9b3478bef51d2f214fb88c31ed3c8d2f4a531ff
2021-06-10 12:58:37 -07:00
Baptiste Lemaire
d61a449364 Fixed manifest_dump issues when printing keys and values containing null characters (#8378)
Summary:
Changed fprintf function to fputc in ApplyVersionEdit, and replaced null characters with whitespaces.
Added unit test in ldb_test.py - verifies that manifest_dump --verbose output is correct when keys and values containing null characters are inserted.

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

Reviewed By: pdillinger

Differential Revision: D29034584

Pulled By: bjlemaire

fbshipit-source-id: 50833687a8a5f726e247c38457eadc3e6dbab862
2021-06-10 12:55:20 -07:00
matthewvon
5a2b4ed671 BugFix: fs_posix.cc GetFreeSpace uses wrong value non-root users (#8370)
Summary:
fs_posix.cc GetFreeSpace() calculates free space based upon a call to statvfs().  However, there are two extremely different values in statvfs's returned structure:  f_bfree which is free space for root and f_bavail which is free space for non-root users.  The existing code uses f_bfree.  Many disks have 5 to 10% of the total disk space reserved for root only.  Therefore GetFreeSpace() does not realize that non-root users may not have storage available.

This PR detects whether the effective posix user is root or not, then selects the appropriate available space value.

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

Reviewed By: mrambacher

Differential Revision: D29032710

Pulled By: jay-zhuang

fbshipit-source-id: 57feba34ed035615a479956d28f98d85735281c0
2021-06-10 11:11:54 -07:00
Zhichao Cao
f44e69c64a Use DbSessionId as cache key prefix when secondary cache is enabled (#8360)
Summary:
Currently, we either use the file system inode or a monotonically incrementing runtime ID as the block cache key prefix. However, if we use a monotonically incrementing runtime ID (in the case that the file system does not support inode id generation), in some cases, it cannot ensure uniqueness (e.g., we have secondary cache migrated from host to host). We use DbSessionID (20 bytes) + current file number (at most 10 bytes) as the new cache block key prefix when the secondary cache is enabled. So can accommodate scenarios such as transfer of cache state across hosts.

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

Test Plan: add the test to lru_cache_test

Reviewed By: pdillinger

Differential Revision: D29006215

Pulled By: zhichao-cao

fbshipit-source-id: 6cff686b38d83904667a2bd39923cd030df16814
2021-06-10 11:02:43 -07:00
Levi Tamasi
db325a5904 Add a clipping internal iterator (#8327)
Summary:
Logically, subcompactions process a key range [start, end); however, the way
this is currently implemented is that the `CompactionIterator` for any given
subcompaction keeps processing key-values until it actually outputs a key that
is out of range, which is then discarded. Instead of doing this, the patch
introduces a new type of internal iterator called `ClippingIterator` which wraps
another internal iterator and "clips" its range of key-values so that any KVs
returned are strictly in the [start, end) interval. This does eliminate a (minor)
inefficiency by stopping processing in subcompactions exactly at the limit;
however, the main motivation is related to BlobDB: namely, we need this to be
able to measure the amount of garbage generated by a subcompaction
precisely and prevent off-by-one errors.

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

Test Plan: `make check`

Reviewed By: siying

Differential Revision: D28761541

Pulled By: ltamasi

fbshipit-source-id: ee0e7229f04edabbc7bed5adb51771fbdc287f69
2021-06-09 15:41:16 -07:00
Peter Dillinger
2f93a3b809 Fix a major performance bug in 6.21 for cache entry stats (#8369)
Summary:
In final polishing of https://github.com/facebook/rocksdb/issues/8297 (after most manual testing), I
broke my own caching layer by sanitizing an input parameter with
std::min(0, x) instead of std::max(0, x). I resisted unit testing the
timing part of the result caching because historically, these test
are either flaky or difficult to write, and this was not a correctness
issue. This bug is essentially unnoticeable with a small number
of column families but can explode background work with a
large number of column families.

This change fixes the logical error, removes some unnecessary related
optimization, and adds mock time/sleeps to the unit test to ensure we
can cache hit within the age limit.

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

Test Plan: added time testing logic to existing unit test

Reviewed By: ajkr

Differential Revision: D28950892

Pulled By: pdillinger

fbshipit-source-id: e79cd4ff3eec68fd0119d994f1ed468c38026c3b
2021-06-08 05:03:32 -07:00
David Devecsery
80a59a03a7 Cancel compact range (#8351)
Summary:
Added the ability to cancel an in-progress range compaction by storing to an atomic "canceled" variable pointed to within the CompactRangeOptions structure.

Tested via two tests added to db_tests2.cc.

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

Reviewed By: ajkr

Differential Revision: D28808894

Pulled By: ddevec

fbshipit-source-id: cb321361c9e23b084b188bb203f11c375a22c2dd
2021-06-07 11:41:31 -07:00
Stepan Koltsov
707f8d168a Modify script which generates TARGETS (#8366)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8366

Test Plan: Run it, `TARGETS` now unchanged.

Reviewed By: jay-zhuang

Differential Revision: D28914138

Pulled By: stepancheg

fbshipit-source-id: 04d24cdf1439edf4204a3ba1f646e9e75a00d92b
2021-06-04 16:28:59 -07:00
Stiopa Koltsov
4d5b575563 Enable Starlark for fbcode//i*
Summary: #forcetdhashing

Reviewed By: ndmitchell

Differential Revision: D28873060

fbshipit-source-id: 7d3be3e7d38619ec5b0b117f462ca1b9f427aa94
2021-06-04 13:19:01 -07:00