Commit Graph

9999 Commits

Author SHA1 Message Date
Zhichao Cao 7457c7cd00 Update release version to 6.19 (#8083)
Summary:
Update release version to 6.19

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

Test Plan: no code change

Reviewed By: riversand963

Differential Revision: D27222083

Pulled By: zhichao-cao

fbshipit-source-id: 94b49997019347e6e6a9e341837f4f9d3149428c
2021-03-21 18:33:46 -07:00
Peter Dillinger 3bfd3ed2f3 Begin forward compatibility for new backup meta schema (#8069)
Summary:
This does not add any new public APIs or published
functionality, but adds the ability to read and use (and in tests,
write) backups with a new meta file schema, based on the old schema
but not forward-compatible (before this change). The new schema enables
some capabilities not in the old:

* Explicit versioning, so that users get clean error messages the next
time we want to break forward compatibility.
* Ignoring unrecognized fields (with warning), so that new non-critical
features can be added without breaking forward compatibility.
* Rejecting future "non-ignorable" fields, so that new features critical
to some use-cases could potentially be added outside of linear schema
versions, with broken forward compatibility.
* Fields at the end of the meta file, such as for checksum of the meta
file's contents (up to that point)
* New optional 'size' field for each file, which is checked when present
* Optionally omitting 'crc32' field, so that we aren't required to have
a crc32c checksum for files to take a backup. (E.g. to support backup
via hard links and to better support file custom checksums.)

Because we do not have a JSON parser and to share code, the new schema
is simply derived from the old schema.

BackupEngine code is updated to allow missing checksums in some places,
and to make that easier, `has_checksum` and `verify_checksum_after_work`
are eliminated. Empty `checksum_hex` indicates checksum is unknown. I'm
not too afraid of regressing on data integrity, because
(a) we have pretty good test coverage of corruption detection in backups, and
(b) we are increasingly relying on the DB itself for data integrity rather than
it being an exclusive feature of backups.

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

Test Plan:
new unit tests, added to crash test (some local run with
boosted backup probability)

Reviewed By: ajkr

Differential Revision: D27139824

Pulled By: pdillinger

fbshipit-source-id: 9e0e4decfb42bb84783d64d2d246456d97e8e8c5
2021-03-19 20:15:40 -07:00
storagezhang c8b0842bcd Remove unused variable (#8067)
Summary:
Remove unused variable `Slice blob_to_write` in `db/blob/blob_file_cache_test.cc`.

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

Reviewed By: zhichao-cao

Differential Revision: D27107693

Pulled By: riversand963

fbshipit-source-id: 9bfd4d296a6a1714ad5c1fa5bb231a0c52dbd56d
2021-03-19 12:13:59 -07:00
storagezhang d9be6556aa Include C++ standard library headers instead of C compatibility headers (#8068)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8068

Reviewed By: zhichao-cao

Differential Revision: D27147685

Pulled By: riversand963

fbshipit-source-id: 5428b1c0142ecae17c977fba31a6d49b52983d1c
2021-03-19 12:09:47 -07:00
storagezhang c706324208 Add default in switch (#8065)
Summary:
switch may not cover all branch in `db/c.cc`:

```c++
void rocksdb_options_set_access_hint_on_compaction_start(
    rocksdb_options_t* opt, int v) {
  switch(v) {
    case 0:
      opt->rep.access_hint_on_compaction_start =
          ROCKSDB_NAMESPACE::Options::NONE;
      break;
    case 1:
      opt->rep.access_hint_on_compaction_start =
          ROCKSDB_NAMESPACE::Options::NORMAL;
      break;
    case 2:
      opt->rep.access_hint_on_compaction_start =
          ROCKSDB_NAMESPACE::Options::SEQUENTIAL;
      break;
    case 3:
      opt->rep.access_hint_on_compaction_start =
          ROCKSDB_NAMESPACE::Options::WILLNEED;
      break;
  }
}
```

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

Reviewed By: riversand963

Differential Revision: D27102892

Pulled By: zhichao-cao

fbshipit-source-id: ad1d20d192712878e61597311ba75b55df0066d7
2021-03-19 11:57:52 -07:00
Zhichao Cao dd0447ae2c Add new Append API with DataVerificationInfo to Env WritableFile (#8071)
Summary:
Add the new Append and PositionedAppend API to env WritableFile. User is able to benefit from the write checksum handoff API when using the legacy Env classes. FileSystem already implemented the checksum handoff API.

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

Test Plan: make check, added new unit test.

Reviewed By: anand1976

Differential Revision: D27177043

Pulled By: zhichao-cao

fbshipit-source-id: 430c8331fc81099fa6d00f4fff703b68b9e8080e
2021-03-19 11:44:13 -07:00
Yanqin Jin 7ee41a5d25 Fix a test failure when built with ASSERT_STATUS_CHECKED=1 (#8075)
Summary:
As title.
Test plan
ASSERT_STATUS_CHECKED=1 make -j20 backupable_db_test error_handler_fs_test
./backupable_db_test
./error_handler_fs_test

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

Reviewed By: zhichao-cao

Differential Revision: D27173832

Pulled By: riversand963

fbshipit-source-id: 37dac50f7c89127804ff2572abddd4174642de30
2021-03-18 21:52:48 -07:00
Yanqin Jin 576cff11da Remove db_with_timestamp_basic_test from platform_dependent list (#8077)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8077

Test Plan: Travis CI

Reviewed By: zhichao-cao

Differential Revision: D27178276

Pulled By: riversand963

fbshipit-source-id: 17911dcc2d5790eb396efcd7f90dea76a127cf15
2021-03-18 20:09:04 -07:00
Yanqin Jin 063a68b9cd Check and handle failure in ldb (#8072)
Summary:
Currently, a few ldb commands do not check the execution result of
database operations. This PR checks the execution results and tries to
improve the error reporting.

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

Test Plan:
```
make check
```
and
```
ASSERT_STATUS_CHECKED=1 make -j20 ldb
python tools/ldb_test.py
```

Reviewed By: zhichao-cao

Differential Revision: D27152466

Pulled By: riversand963

fbshipit-source-id: b94220496a4b3591b61c1d350f665860a6579f30
2021-03-18 14:43:34 -07:00
Zhichao Cao c810947184 Separate handling of WAL Sync io error with SST flush io error (#8049)
Summary:
In previous codebase, if WAL is used, all the retryable IO Error will be treated as hard error. So write is stalled. In this PR, the retryable IO error from WAL sync is separated from SST file flush io error. If WAL Sync is ok and retryable IO Error only happens during SST flush, the error is mapped to soft error. So user can continue insert to Memtable and append to WAL.

Resolve the bug that if WAL sync fails, the memtable status does not roll back due to calling PickMemtable early than calling and checking SyncClosedLog.

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

Test Plan: added new unit test, make check

Reviewed By: anand1976

Differential Revision: D26965529

Pulled By: zhichao-cao

fbshipit-source-id: f5fecb66602212523c92ee49d7edcb6065982410
2021-03-18 14:33:16 -07:00
Peter Dillinger e7a60d01b2 Revamp WriteController (#8064)
Summary:
WriteController had a number of issues:
* It could introduce a delay of 1ms even if the write rate never exceeded the
configured delayed_write_rate.
* The DB-wide delayed_write_rate could be exceeded in a number of ways
with multiple column families:
  * Wiping all pending delay "debts" when another column family joins
  the delay with GetDelayToken().
  * Resetting last_refill_time_ to (now + sleep amount) means each
  column family can write with delayed_write_rate for large writes.
  * Updating bytes_left_ for a partial refill without updating
  last_refill_time_ would essentially give out random bonuses,
  especially to medium-sized writes.

Now the code is much simpler, with these issues fixed. See comments in
the new code and new (replacement) tests.

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

Test Plan: new tests, better than old tests

Reviewed By: mrambacher

Differential Revision: D27064936

Pulled By: pdillinger

fbshipit-source-id: 497c23fe6819340b8f3d440bd634d8a2bc47323f
2021-03-18 09:47:31 -07:00
Zhichao Cao 08ec5e7321 Add the statistics and info log for Error handler (#8050)
Summary:
Add statistics and info log for error handler: counters for bg error, bg io error, bg retryable io error, auto resume, auto resume total retry, and auto resume sucess; Histogram for auto resume retry count in each recovery call.

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

Test Plan: make check and add test to error_handler_fs_test

Reviewed By: anand1976

Differential Revision: D26990565

Pulled By: zhichao-cao

fbshipit-source-id: 49f71e8ea4e9db8b189943976404205b56ab883f
2021-03-17 22:38:13 -07:00
Akanksha Mahajan 27d57a035e Use SST file manager to track blob files as well (#8037)
Summary:
Extend support to track blob files in SST File manager.
 This PR notifies SstFileManager whenever a new blob file is created,
 via OnAddFile and  an obsolete blob file deleted via OnDeleteFile
 and delete file via ScheduleFileDeletion.

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

Test Plan: Add new unit tests

Reviewed By: ltamasi

Differential Revision: D26891237

Pulled By: akankshamahajan15

fbshipit-source-id: 04c69ccfda2a73782fd5c51982dae58dd11979b6
2021-03-17 20:44:49 -07:00
Xiaopeng Zhang c603f2f898 support getUsage and getPinnedUsage in JavaAPI for Cache (#7925)
Summary:
support getUsage and getPinnedUsage in JavaAPI for Cache
also fix a typo in LRUCacheTest.java that the highPriPoolRatio is not valid(set 5, I guess it means 0.05)

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

Reviewed By: mrambacher

Differential Revision: D26900241

Pulled By: ajkr

fbshipit-source-id: 735d1e40a16fa8919c89c7c7154ba7f81208ec33
2021-03-17 09:30:33 -07:00
Mark Callaghan 326670d265 Add new db_bench --benchmarks options for controlling compaction (#8027)
Summary:
The new options are:
* compact0 - compact L0 into L1 using one thread
* compact1 - compact L1 into L2 using one thread
* flush - flush memtable
* waitforcompaction - wait for compaction to finish

These are useful for reproducible benchmarks to help get the LSM tree shape
into a deterministic state. I wrote about this at:
http://smalldatum.blogspot.com/2021/02/read-only-benchmarks-with-lsm-are.html

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

Reviewed By: riversand963

Differential Revision: D27053861

Pulled By: ajkr

fbshipit-source-id: 1646f35584a3db03740fbeb47d91c3f00fb35d6e
2021-03-17 09:12:27 -07:00
stefan-zobel 8d9088464b Java-API: Fix minor Javadoc copy-paste errors (#8034)
Summary:
Fixes 3 minor Javadoc copy-paste errors in the `RocksDB#newIterator()` and `Transaction#getIterator()` variants that take a column family handle but are talking about iterating over "the database" or "the default column family".

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

Reviewed By: jay-zhuang

Differential Revision: D26877667

Pulled By: mrambacher

fbshipit-source-id: 95dd95b667c496e389f221acc9a91b340e4b63bf
2021-03-16 18:07:09 -07:00
mrambacher 1a343bc393 Make ChRootEnv, EncryptedEnv, and TimedEnv into FileSystems (#7968)
Summary:
These classes were wraps of Env that provided only extensions to the FileSystem functionality.  Changed the classes to be FileSystems and the wraps to be of the CompositeEnvWrapper.

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

Reviewed By: anand1976

Differential Revision: D26900253

Pulled By: mrambacher

fbshipit-source-id: 94001d8024a3c54a1c11adadca2bac66c3af2a77
2021-03-15 19:50:11 -07:00
Yanqin Jin 0304352882 Fix a bug in key comparison when index type is kBinarySearchWithFirstKey (#8062)
Summary:
When timestamp is enabled, key comparison should take this into account.
In `BlockBasedTableReader::Get()`, `BlockBasedTableReader::MultiGet()`,
assume the target key is `key`, and the timestamp upper bound is `ts`.
The highest key in current block is (key, ts1), while the lowest key in next
block is (key, ts2).
If
```
ts1 > ts > ts2
```
then
```
(key, ts1) < (key, ts) < (key, ts2)
```
It can be shown that if `Compare()` is used, then we will mistakenly skip the next
block. Instead, we should use `CompareWithoutTimestamp()`.

The majority of this PR makes some existing tests in `db_with_timestamp_basic_test.cc`
parameterized so that different index types can be tested. A new unit test is
also added for more coverage.

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

Test Plan: make check

Reviewed By: ltamasi

Differential Revision: D27057557

Pulled By: riversand963

fbshipit-source-id: c1062fa7c159ed600a1ad7e461531d52265021f1
2021-03-15 17:44:52 -07:00
Yanqin Jin 85d4f2c8b3 Move a test file to a better location (#8054)
Summary:
As title.

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

Test Plan: make check

Reviewed By: mrambacher

Differential Revision: D27017955

Pulled By: riversand963

fbshipit-source-id: 829497d507bc89afbe982f8a8cf3555e52fd7098
2021-03-15 15:03:27 -07:00
mrambacher 3dff28cf9b Use SystemClock* instead of std::shared_ptr<SystemClock> in lower level routines (#8033)
Summary:
For performance purposes, the lower level routines were changed to use a SystemClock* instead of a std::shared_ptr<SystemClock>.  The shared ptr has some performance degradation on certain hardware classes.

For most of the system, there is no risk of the pointer being deleted/invalid because the shared_ptr will be stored elsewhere.  For example, the ImmutableDBOptions stores the Env which has a std::shared_ptr<SystemClock> in it.  The SystemClock* within the ImmutableDBOptions is essentially a "short cut" to gain access to this constant resource.

There were a few classes (PeriodicWorkScheduler?) where the "short cut" property did not hold.  In those cases, the shared pointer was preserved.

Using db_bench readrandom perf_level=3 on my EC2 box, this change performed as well or better than 6.17:

6.17: readrandom   :      28.046 micros/op 854902 ops/sec;   61.3 MB/s (355999 of 355999 found)
6.18: readrandom   :      32.615 micros/op 735306 ops/sec;   52.7 MB/s (290999 of 290999 found)
PR: readrandom   :      27.500 micros/op 871909 ops/sec;   62.5 MB/s (367999 of 367999 found)

(Note that the times for 6.18 are prior to revert of the SystemClock).

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

Reviewed By: pdillinger

Differential Revision: D27014563

Pulled By: mrambacher

fbshipit-source-id: ad0459eba03182e454391b5926bf5cdd45657b67
2021-03-15 04:34:11 -07:00
Andrew Kryczka b8f40f7f7b Deflake tests of compaction based on compensated file size (#8036)
Summary:
CompactionDeletionTriggerReopen was observed to be flaky recently:
https://app.circleci.com/pipelines/github/facebook/rocksdb/6030/workflows/787af4f3-b9f7-4645-8e8d-1fb0ebf05539/jobs/101451.

I went through it and the related tests and arrived at different
conclusions on what constraints we can expect on DB size. Some
constraints got looser and some got tighter. The particular constraint
that flaked got a lot looser so at least the flake linked above would have been prevented.

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

Reviewed By: riversand963

Differential Revision: D26862566

Pulled By: ajkr

fbshipit-source-id: 3512b86b4fb41aeecae32e1c7382c03916d88d88
2021-03-14 20:25:42 -07:00
Levi Tamasi b708b166dc Fix a harmless data race affecting two test cases (#8055)
Summary:
`DBTest.GetLiveBlobFiles` and `ObsoleteFilesTest.BlobFiles` both modify the
current `Version` in their setup phase, implicitly assuming that no other
threads would touch the `Version` while this is happening. The periodic
stats dumper thread violates this assumption; the patch fixes this by
disabling it in the affected test cases. (Note: the data race is
harmless in the sense that it only affects test code.)

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

Test Plan:
```
COMPILE_WITH_TSAN=1 make db_test -j24
gtest-parallel --repeat=10000 ./db_test --gtest_filter="*GetLiveBlobFiles"
COMPILE_WITH_TSAN=1 make obsolete_files_test -j24
gtest-parallel --repeat=10000 ./obsolete_files_test --gtest_filter="*BlobFiles"
```

Reviewed By: riversand963

Differential Revision: D27022715

Pulled By: ltamasi

fbshipit-source-id: b6cc77ed63d8bc1cbe0603522ff1a572182fc9ab
2021-03-12 16:44:35 -08:00
Peter Dillinger 01c2ec3fcb Add ROCKSDB_GTEST_BYPASS (#8048)
Summary:
This is for cases that do not meet the Facebook criteria for
SKIP (see new comments). Also made ROCKSDB_GTEST_{SKIP,BYPASS} print the
message because gtest doesn't ever seem to.

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

Test Plan: manual inspection of ./ribbon_test output, CI

Reviewed By: mrambacher

Differential Revision: D26953688

Pulled By: pdillinger

fbshipit-source-id: c914eaffe7d419db6ab90a193d474531e23582e5
2021-03-12 16:02:06 -08:00
Peter Dillinger 119dda2195 Instantiate tests DBIteratorTestForPinnedData (#8051)
Summary:
a trial gtest upgrade discovered some parameterized tests missing instantiation. By some miracle, they still pass.

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

Test Plan: thisisthetest

Reviewed By: mrambacher

Differential Revision: D27003684

Pulled By: pdillinger

fbshipit-source-id: cde1cab1551fb282f67d462d46574bd30bd5e61f
2021-03-12 12:31:29 -08:00
Peter Dillinger 589ea6bec2 Add BackupEngine API for backup file details (#8042)
Summary:
This API can be used for things like determining how much space
can be freed up by deleting a particular backup, etc.

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

Test Plan:
validation of the API added to many existing backup unit
tests

Reviewed By: mrambacher

Differential Revision: D26936577

Pulled By: pdillinger

fbshipit-source-id: f0bbd90f0917b9781a6837652fb4616d9247816a
2021-03-12 11:03:54 -08:00
Yanqin Jin 82b3888433 Enable backward iterator for keys with user-defined timestamp (#8035)
Summary:
This PR does the following:

- Enable backward iteration for keys with user-defined timestamp. Note that merge, single delete, range delete are not supported yet.
- Introduces a new helper API `Comparator::EqualWithoutTimestamp()`.
- Fix a typo in `SetTimestamp()`.
- Add/update unit tests

Run db_bench (built with DEBUG_LEVEL=0) to demonstrate that no overhead is introduced for CPU-intensive workloads with a lot of `Prev()`. Also provided results of iterating keys with timestamps.

1. Disable timestamp, run:
```
./db_bench -db=/dev/shm/rocksdb -disable_wal=1 -benchmarks=fillseq,seekrandom[-W1-X6] -reverse_iterator=1 -seek_nexts=5
```
Results:
> Baseline
> - seekrandom [AVG    6 runs] : 96115 ops/sec;   53.2 MB/sec
> - seekrandom [MEDIAN 6 runs] : 98075 ops/sec;   54.2 MB/sec
>
> This PR
> - seekrandom [AVG    6 runs] : 95521 ops/sec;   52.8 MB/sec
> - seekrandom [MEDIAN 6 runs] : 96338 ops/sec;   53.3 MB/sec

2. Enable timestamp, run:
```
./db_bench -user_timestamp_size=8  -db=/dev/shm/rocksdb -disable_wal=1 -benchmarks=fillseq,seekrandom[-W1-X6] -reverse_iterator=1 -seek_nexts=5
```
Result:
> Baseline: not supported
>
> This PR
> - seekrandom [AVG    6 runs] : 90514 ops/sec;   50.1 MB/sec
> - seekrandom [MEDIAN 6 runs] : 90834 ops/sec;   50.2 MB/sec

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

Reviewed By: ltamasi

Differential Revision: D26926668

Pulled By: riversand963

fbshipit-source-id: 95330cc2242397c03e09d29e5417dfb0adc98ef5
2021-03-10 11:15:46 -08:00
Yanqin Jin 64517d184a Make secondary instance use ManifestTailer (#7998)
Summary:
This PR

- adds a class `ManifestTailer` that inherits from `VersionEditHandlerPointInTime`. `ManifestTailer::Iterate()` can be called multiple times to tail the primary instance's MANIFEST and apply the changes to the secondary,
- updates the implementation of `ReactiveVersionSet::ReadAndApply` to use this class,
- removes unused code in version_set.cc,
- updates existing tests, e.g. removing deleted sync points from unit tests,
- adds a new test to address the bug in https://github.com/facebook/rocksdb/issues/7815.

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

Test Plan:
make check
Existing and newly-added tests in version_set_test.cc and db_secondary_test.cc

Reviewed By: jay-zhuang

Differential Revision: D26926641

Pulled By: riversand963

fbshipit-source-id: 8d4dd15db0ba863c213f743e33b5a207e948c980
2021-03-10 10:59:44 -08:00
David CARLIER 7a3444bf1f Mac M1 crc32 intrinsics ARM64 check support proposal (#7893)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7893

Reviewed By: ajkr

Differential Revision: D26050966

Pulled By: jay-zhuang

fbshipit-source-id: 9df2bb65d82defd7fad49d5369979b03e22d39c2
2021-03-10 09:05:56 -08:00
stefan-zobel cc34da75b5 Java-API: byteCompressionType should be declared as primitive type byte (#7981)
Summary:
The variable `byteCompressionType` is only assigned values of primitive type and is never 'null', but it is declared with the boxed type 'Byte'.

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

Reviewed By: ajkr

Differential Revision: D26546600

Pulled By: jay-zhuang

fbshipit-source-id: 07b579cdfcfc2262a448ca3626e216416fd05892
2021-03-09 22:05:16 -08:00
qinzuoyan 6fad38ebe8 Fix compile error (#7908)
Summary:
OS: Ubuntu 14.04
Compiler: GCC 4.9.4
Compile error:
```
db/forward_iterator.cc:996:62: error: declaration of ‘key’ shadows a member of 'this' [-Werror=shadow]
   auto cmp = [&](const FileMetaData* f, const Slice& key) -> bool {
```

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

Reviewed By: jay-zhuang

Differential Revision: D26899986

Pulled By: ajkr

fbshipit-source-id: 66b0b97aefd0f13a085e063491f8207366a9f848
2021-03-09 20:53:33 -08:00
Hans Holmberg 670567db09 Add support for custom file systems to ldb and sst_dump (#8010)
Summary:
This PR adds support for custom file systems to ldb and sst_dump by adding command line options for specifying --fs_uri and --backup_fs uri (for ldb backup/restore commands). fs_uri is already supported in db_bench and db_stress, and there is already support in ldb and db stress for specifying customized envs.

The PR also fixes what looks like a bug in the ldb backup/restore commands. As it is right now, backups can only be made from and to the same environment/file system which does not seem to be the intended behavior. This PR makes it possible to do/restore backups between different envs/file systems.

Example:
`./ldb backup --fs_uri=zenfs://dev:nvme2n1 --backup_fs_uri=posix:// --backup_dir=/tmp/my_rocksdb_backup  --db=rocksdbtest/dbbench
`

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

Reviewed By: jay-zhuang

Differential Revision: D26904654

Pulled By: ajkr

fbshipit-source-id: 9b695ed8b944fcc6b27c4daaa9f52e87ee2c1fb4
2021-03-09 20:49:15 -08:00
Ed rodriguez 7381dad1b1 make:Fix c header prototypes (#7994)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7994

Reviewed By: jay-zhuang

Differential Revision: D26904603

Pulled By: ajkr

fbshipit-source-id: 0af92a51de895b40c7faaa4f0870b3f63279fe21
2021-03-09 20:44:23 -08:00
Peter Dillinger 4b18c46d10 Refactor: add LineFileReader and Status::MustCheck (#8026)
Summary:
Removed confusing, awkward, and undocumented internal API
ReadOneLine and replaced with very simple LineFileReader.

In refactoring backupable_db.cc, this has the side benefit of
removing the arbitrary cap on the size of backup metadata files.

Also added Status::MustCheck to make it easy to mark a Status as
"must check." Using this, I can ensure that after
LineFileReader::ReadLine returns false the caller checks GetStatus().

Also removed some excessive conditional compilation in status.h

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

Test Plan: added unit test, and running tests with ASSERT_STATUS_CHECKED

Reviewed By: mrambacher

Differential Revision: D26831687

Pulled By: pdillinger

fbshipit-source-id: ef749c265a7a26bb13cd44f6f0f97db2955f6f0f
2021-03-09 20:12:38 -08:00
Peter Dillinger 847ca9f964 Make default share_files_with_checksum=true (#8020)
Summary:
New comment for share_files_with_checksum:
// Only used if share_table_files is set to true. Setting to false is
// DEPRECATED and potentially dangerous because in that case BackupEngine
// can lose data if backing up databases with distinct or divergent
// history, for example if restoring from a backup other than the latest,
// writing to the DB, and creating another backup. Setting to true (default)
// prevents these issues by ensuring that different table files (SSTs) with
// the same number are treated as distinct. See
// share_files_with_checksum_naming and ShareFilesNaming.

I have also removed interim option kFlagMatchInterimNaming, which is no
longer needed and was never needed for correct+compatible operation
(just performance).

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

Test Plan:
tests updated. Backward+forward compatibility verified with
SHORT_TEST=1 check_format_compatible.sh. ldb uses default backup
options, and I manually verified shared_checksum in
/tmp/rocksdb_format_compatible_peterd/bak/current/ after run.

Reviewed By: ajkr

Differential Revision: D26786331

Pulled By: pdillinger

fbshipit-source-id: 36f968dfef1f5cacbd65154abe1d846151a55130
2021-03-09 16:27:13 -08:00
Peter Dillinger 0028e3398b Make format_version=5 new default (#8017)
Summary:
Haven't seen any production issues with new Bloom filter and
it's now > 1 year old (added in 6.6.0).

Updated check_format_compatible.sh and HISTORY.md

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

Test Plan: tests updated (or prior bugs fixed)

Reviewed By: ajkr

Differential Revision: D26762197

Pulled By: pdillinger

fbshipit-source-id: 0e755c46b443087c1544da0fd545beb9c403d1c2
2021-03-09 12:42:53 -08:00
stefan-zobel 430842f948 Java-API: Missing space in string literal (#7982)
Summary:
`TtlDB.open()`: missing space after 'column'
`AdvancedColumnFamilyOptionsInterface.setLevelCompactionDynamicLevelBytes()`: missing space after 'cause'

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

Reviewed By: ajkr

Differential Revision: D26546632

Pulled By: jay-zhuang

fbshipit-source-id: 885dedcaa2200842764fbac9ce3766d54e1c8914
2021-03-09 11:30:29 -08:00
xinyuliu 8643d63bb4 Add $(ARTIFACT_SUFFIX} to benchmark tools built with cmake (#8016)
Summary:
Add ${ARTIFACT_SUFFIX} to benchmark tool names to enable differentiating jemalloc and non-jemalloc versions.

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

Reviewed By: jay-zhuang

Differential Revision: D26907007

Pulled By: ajkr

fbshipit-source-id: 78d3b3372b5454d52d5b663ea982135ea9cf7bf8
2021-03-09 10:38:22 -08:00
fanrui03 67d72fb5dc Fix checkpoint stuck (#7921)
Summary:
## 1. Bug description:

When RocksDB Checkpoint, it may be stuck in `WaitUntilFlushWouldNotStallWrites` method.

## 2. Simple analysis of the reasons:

### 2.1 Configuration parameters:

```yaml
Compaction Style : Universal

max_write_buffer_number : 4
min_write_buffer_number_to_merge : 3
```

Checkpoint is usually very fast. When the Checkpoint is executed, `WaitUntilFlushWouldNotStallWrites` is called. If there are 2 Immutable MemTables, which are less than `min_write_buffer_number_to_merge`, they will not be flushed. But will enter this code.

```c++
// method: GetWriteStallConditionAndCause
if (mutable_cf_options.max_write_buffer_number> 3 &&
              num_unflushed_memtables >=
                  mutable_cf_options.max_write_buffer_number-1) {
     return {WriteStallCondition::kDelayed, WriteStallCause::kMemtableLimit};
}
```

code link: fbed72f03c/db/column_family.cc (L847)

Checkpoint thought there was a FlushJob, but it didn't. So will always wait.

### 2.2 solution:

Increase the restriction: the `number of Immutable MemTable` >= `min_write_buffer_number_to_merge will wait`.

If there are other better solutions, you can correct me.

### 2.3 Code that can reproduce the problem:

https://github.com/1996fanrui/fanrui-learning/blob/flink-1.12/module-java/src/main/java/com/dream/rocksdb/RocksDBCheckpointStuck.java

## 3. Interesting point

This bug will be triggered only when `the number of sorted runs >= level0_file_num_compaction_trigger`.

Because there is a break in WaitUntilFlushWouldNotStallWrites.

```c++
if (cfd->imm()->NumNotFlushed() <
        cfd->ioptions()->min_write_buffer_number_to_merge &&
    vstorage->l0_delay_trigger_count() <
        mutable_cf_options.level0_file_num_compaction_trigger) {
  break;
}
```

code link: fbed72f03c/db/db_impl/db_impl_compaction_flush.cc (L1974)

Universal may have `l0_delay_trigger_count() >= level0_file_num_compaction_trigger`, so this bug is triggered.

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

Reviewed By: jay-zhuang

Differential Revision: D26900559

Pulled By: ajkr

fbshipit-source-id: 133c1252dad7393753f04a47590b68c7d8e670df
2021-03-09 02:21:25 -08:00
kshair d2e9eab1ea Fix mis-spelling (#8001)
Summary:
concurrnet -> concurrent

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

Reviewed By: ajkr

Differential Revision: D26659381

Pulled By: riversand963

fbshipit-source-id: 890d102d1cf836ed3b183da66d3d56a3158017d0
2021-03-09 01:19:18 -08:00
jsteemann 02974c9437 make PerfStepTimer struct smaller by reordering members (#7931)
Summary:
On x86_64, this makes the struct 8 bytes smaller, so creating a PerfStepTimer on the stack will use slightly less stack space.

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

Reviewed By: jay-zhuang

Differential Revision: D26529470

Pulled By: ajkr

fbshipit-source-id: bbe2e843167152ffa05a5946f1add6621c9849f7
2021-03-08 21:33:15 -08:00
Andrew Kryczka ef392fb04e use `LIB_MODE=shared` on Travis `make` commands (#8043)
Summary:
We were seeing intermittent `ld` failures due to `No space left on device` such as https://travis-ci.org/github/facebook/rocksdb/jobs/761905070.

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

Reviewed By: pdillinger

Differential Revision: D26889711

Pulled By: ajkr

fbshipit-source-id: 010b7617d339bddc30026586bfde41539632fb2d
2021-03-08 17:21:24 -08:00
Andrew Kryczka 0ff0b625a1 Deflake DBTest2.PartitionedIndexUserToInternalKey on ppc64le (#8044)
Summary:
For some reason I still cannot figure out, the manual flush in this test
was sometimes producing a third tiny file. I saw it a bunch of times on
ppc64le, but even running a qemu system with that architecture (and
playing with various other options) could not repro. However we did get
an instrumented Travis run to confirm the problem is indeed a third tiny
file - https://travis-ci.org/github/facebook/rocksdb/jobs/761986592. We
can avoid it by filling memtables less full and using manual flush.

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

Reviewed By: akankshamahajan15

Differential Revision: D26892635

Pulled By: ajkr

fbshipit-source-id: 775c04176931cf01d07cc78fb82cfe3a11beebcf
2021-03-08 14:47:56 -08:00
Peter Dillinger ce391ff84b Clarifying comments for Read() APIs (#8029)
Summary:
I recently discovered the confusing, undocumented semantics of
Read() functions in the FileSystem and Env APIs. I have added
clarification to the best of my reverse-engineered understanding, and
made a note in HISTORY.md for implementors to check their
implementations, as a subtly non-adherent implementation could lead to
RocksDB quietly ignoring some portion of a file.

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

Test Plan: no code changes

Reviewed By: anand1976

Differential Revision: D26831698

Pulled By: pdillinger

fbshipit-source-id: 208f97ff6037bc13bb2ef360b987c2640c79bd03
2021-03-05 14:42:19 -08:00
Levi Tamasi cb25bc1128 Update compaction statistics to include the amount of data read from blob files (#8022)
Summary:
The patch does the following:
1) Exposes the amount of data (number of bytes) read from blob files from
`BlobFileReader::GetBlob` / `Version::GetBlob`.
2) Tracks the total number and size of blobs read from blob files during a
compaction (due to garbage collection or compaction filter usage) in
`CompactionIterationStats` and propagates this data to
`InternalStats::CompactionStats` / `CompactionJobStats`.
3) Updates the formulae for write amplification calculations to include the
amount of data read from blob files.
4) Extends the compaction stats dump with a new column `Rblob(GB)` and
a new line containing the total number and size of blob files in the current
`Version` to complement the information about the shape and size of the LSM tree
that's already there.
5) Updates `CompactionJobStats` so that the number of files and amount of data
written by a compaction are broken down per file type (i.e. table/blob file).

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

Test Plan: Ran `make check` and `db_bench`.

Reviewed By: riversand963

Differential Revision: D26801199

Pulled By: ltamasi

fbshipit-source-id: 28a5f072048a702643b28cb5971b4099acabbfb2
2021-03-04 00:43:48 -08:00
matthewvon 4126bdc0e1 Feature: add SetBufferSize() so that managed size can be dynamic (#7961)
Summary:
This PR adds SetBufferSize() to the WriteBufferManager object.  This enables user code to adjust the global budget for write_buffers based upon other memory conditions such as growth in table reader memory as the dataset grows.

The buffer_size_ member variable is now atomic to match design of other changeable size_t members within WriteBufferManager.

This change is useful as is.  However, this change is also essential if someone decides they wanted to enable db_write_buffer_size modifications through the DB::SetOptions() API, i.e. no waste taking this as is.

Any format / spacing changes are due to clang-format as required by check-in automation.

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

Reviewed By: ajkr

Differential Revision: D26639075

Pulled By: akankshamahajan15

fbshipit-source-id: 0604348caf092d35f44e85715331dc920e5c1033
2021-03-03 14:22:11 -08:00
Yanqin Jin 72d1e258cd Possibly bump NUMBER_OF_RESEEKS_IN_ITERATION (#8015)
Summary:
When changing db iterator direction, we may perform a reseek.
Therefore, we should bump the NUMBER_OF_RESEEKS_IN_ITERATION counter.

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

Test Plan: make check

Reviewed By: ltamasi

Differential Revision: D26755415

Pulled By: riversand963

fbshipit-source-id: 211f51f1a454bcda768fc46c0dce51edeb7f05fe
2021-03-02 22:41:04 -08:00
Peter Dillinger a9046f3c45 Revamp check_format_compatible.sh (#8012)
Summary:
* Adds backup/restore forward/backward compatibility testing
* Adds forward/backward compatibility testing to sst ingestion
* More structure sharing and comments for the lists of branches
comprising each group
* Less reliant on invariants between groups with de-duplication logic
* Restructured for n+1 branch checkout+build steps rather than something
like 3n. Should be much faster despite more checks.

And to make manual runs easier

* On success, restores working trees to original working branch (aborts
early if uncommitted changes) and deletes temporary branch & remote
* Adds SHORT_TEST=1 mode that uses only the oldest version for each
* Adds USE_SSH=1 to use ssh instead of https for github
group

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

Test Plan:
a number of manual tests, mostly with SHORT_TEST=1. Using one
version older for any of the groups (except I didn't check
db_backward_only_refs) fails. Changing default format_version to 5
(planned) without updating this script fails as it should, and passes
with appropriate update. Full local run passed (had to remove "2.7.fb.branch"
due to compiler issues, also before this change).

Reviewed By: riversand963

Differential Revision: D26735840

Pulled By: pdillinger

fbshipit-source-id: 1320c22de5674760657e385aa42df9fade8b6fff
2021-03-02 11:42:27 -08:00
Levi Tamasi a46f080cce Break down the amount of data written during flushes/compactions per file type (#8013)
Summary:
The patch breaks down the "bytes written" (as well as the "number of output files")
compaction statistics into two, so the values are logged separately for table files
and blob files in the info log, and are shown in separate columns (`Write(GB)` for table
files, `Wblob(GB)` for blob files) when the compaction statistics are dumped.
This will also come in handy for fixing the write amplification statistics, which currently
do not consider the amount of data read from blob files during compaction. (This will
be fixed by an upcoming patch.)

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

Test Plan: Ran `make check` and `db_bench`.

Reviewed By: riversand963

Differential Revision: D26742156

Pulled By: ltamasi

fbshipit-source-id: 31d18ee8f90438b438ca7ed1ea8cbd92114442d5
2021-03-02 09:48:00 -08:00
Akanksha Mahajan f19612970d Support retrieving checksums for blob files from the MANIFEST when checkpointing (#8003)
Summary:
The checkpointing logic supports passing file level checksums
to the copy_file_cb callback function which is used by the backup code
for detecting corruption during file copies.
However, this is currently implemented only for table files.

This PR extends the checksum retrieval to blob files as well.

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

Test Plan: Add new test units

Reviewed By: ltamasi

Differential Revision: D26680701

Pulled By: akankshamahajan15

fbshipit-source-id: 1bd1e2464df6e9aa31091d35b8c72786d94cd1c5
2021-03-01 20:07:07 -08:00
Yanqin Jin 1f11d07f24 Enable compact filter for blob in dbstress and dbbench (#8011)
Summary:
As title.

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

Test Plan:
```
./db_bench -enable_blob_files=1 -use_keep_filter=1 -disable_auto_compactions=1
/db_stress -enable_blob_files=1 -enable_compaction_filter=1 -acquire_snapshot_one_in=0 -compact_range_one_in=0 -iterpercent=0 -test_batches_snapshots=0 -readpercent=10 -prefixpercent=20 -writepercent=55 -delpercent=15 -continuous_verification_interval=0
```

Reviewed By: ltamasi

Differential Revision: D26736061

Pulled By: riversand963

fbshipit-source-id: 1c7834903c28431ce23324c4f259ed71255614e2
2021-03-01 17:24:47 -08:00