Commit graph

9175 commits

Author SHA1 Message Date
Akanksha Mahajan 2677bd5967 Add logs and stats in DeleteScheduler (#6927)
Summary:
Add logs and stats for files marked as trash and files deleted immediately in DeleteScheduler
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6927

Test Plan: make check -j64

Reviewed By: riversand963

Differential Revision: D21869068

Pulled By: akankshamahajan15

fbshipit-source-id: e9f673c4fa8049ce648b23c75d742f2f9c6c57a1
2020-06-05 09:43:04 -07:00
Peter Dillinger aaece2a98d Fix some defects reported by Coverity Scan (#6933)
Summary:
Confusing checks for null that are never null
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6933

Test Plan: make check

Reviewed By: cheng-chang

Differential Revision: D21885466

Pulled By: pdillinger

fbshipit-source-id: 4b48e03c2a33727f2702b0d12292f9fda5a3c475
2020-06-04 15:46:27 -07:00
Peter Dillinger c7432cc3c0 Fix more defects reported by Coverity Scan (#6935)
Summary:
Mostly uninitialized values: some probably written before use, but some seem like bugs. Also, destructor needs to be virtual, and possible use-after-free in test
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6935

Test Plan: make check

Reviewed By: siying

Differential Revision: D21885484

Pulled By: pdillinger

fbshipit-source-id: e2e7cb0a0cf196f2b55edd16f0634e81f6cc8e08
2020-06-04 15:35:08 -07:00
Yanqin Jin a8170d774c Close file to avoid file-descriptor leakage (#6936)
Summary:
When operation on an open file descriptor fails, we should close the file descriptor.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6936

Test Plan: make check

Reviewed By: pdillinger

Differential Revision: D21885458

Pulled By: riversand963

fbshipit-source-id: ba077a76b256a8537f21e22e4ec198f45390bf50
2020-06-04 14:21:15 -07:00
sdong 6cbe9d9762 Make StringAppendOperatorTest a parameterized test (#6930)
Summary:
StringAppendOperatorTest right now runs in a mode where RUN_ALL_TESTS() is executed twice for the same test but different settings. This creates a problem with a tool that expects every test to run once. Fix it by using a parameterized test instead.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6930

Test Plan: Run the test and see it passed.

Reviewed By: ltamasi

Differential Revision: D21874145

fbshipit-source-id: 55520b2d7f1ba9f3cba1e2d087fe86f43fb06145
2020-06-04 14:17:11 -07:00
sdong 31bd2d790e Fix ThreadLocalTest.SequentialReadWriteTest failure when running individually (#6929)
Summary:
When running ThreadLocalTest.SequentialReadWriteTest individually, the test fails with:

] ./thread_local_test --gtest_filter="*SequentialReadWriteTest*"
Note: Google Test filter = *SequentialReadWriteTest*
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ThreadLocalTest
[ RUN      ] ThreadLocalTest.SequentialReadWriteTest
internal_repo_rocksdb/repo/util/thread_local_test.cc:144: Failure
      Expected: IDChecker::PeekId()
      Which is: 3
To be equal to: base_id + 1u
      Which is: 2
[  FAILED  ] ThreadLocalTest.SequentialReadWriteTest (1 ms)
[----------] 1 test from ThreadLocalTest (1 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (1 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ThreadLocalTest.SequentialReadWriteTest

 1 FAILED TEST

It appears that when running as the first test, PeakId() was updated twice. I didn't dig into it why but it doesn't seem to break the contract. Relax the assertion to make it pass.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6929

Test Plan: Run the test individually and as the whole thread_local_test

Reviewed By: riversand963

Differential Revision: D21873999

fbshipit-source-id: 1dcb6a2e9c38b6afd848027308bfe633342b7548
2020-06-04 11:44:09 -07:00
mrambacher e85cbdb4e8 Fix two core dumps when files are missing (#6922)
Summary:
The LDB create and drop column family commands failed to check if theere was a valid database prior to dereferencing it, leading to a core dump.

The SstFileDumper prefetch code would dereference a file when the file did not exist as part of the Prefetch code.  This dereference was moved inside an st.ok() check.

Tests were added for both failure conditions.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6922

Reviewed By: gg814

Differential Revision: D21884024

Pulled By: pdillinger

fbshipit-source-id: bddd45c299aa9dc7e928c17a37a96521f8c9149e
2020-06-04 11:40:32 -07:00
sdong 0b45a68c59 env_test */RunMany/* tests to run individually (#6931)
Summary:
When run */RunMany/* tests individually, e.g. ChrootEnvWithDirectIO/EnvPosixTestWithParam.RunMany/0, they hang. It's because they insert to background thread pool without initializing them. Fix it.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6931

Test Plan: Run ChrootEnvWithDirectIO/EnvPosixTestWithParam.RunMany/0 by itself and see it passes.

Reviewed By: riversand963

Differential Revision: D21875603

fbshipit-source-id: 7f848174c1a660254a2b1f7e11cca5370793ba30
2020-06-04 09:51:38 -07:00
Yanqin Jin 2f3261831b Fix a typo (bug) when setting error during Flush (#6928)
Summary:
As title. The prior change to the line is a typo. Fixing it.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6928

Test Plan: make check

Reviewed By: zhichao-cao

Differential Revision: D21873587

Pulled By: riversand963

fbshipit-source-id: f4837fc8792d7106bc230b7b499dfbb7a2847430
2020-06-04 08:30:42 -07:00
Zitan Chen 02df00d97b API change: DB::OpenForReadOnly will not write to the file system unless create_if_missing is true (#6900)
Summary:
DB::OpenForReadOnly will not write anything to the file system (i.e., create directories or files for the DB) unless create_if_missing is true.

This change also fixes some subcommands of ldb, which write to the file system even if the purpose is for readonly.

Two tests for this updated behavior of DB::OpenForReadOnly are also added.

Other minor changes:
1. Updated HISTORY.md to include this API change of DB::OpenForReadOnly;
2. Updated the help information for the put and batchput subcommands of ldb with the option [--create_if_missing];
3. Updated the comment of Env::DeleteDir to emphasize that it returns OK only if the directory to be deleted is empty.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6900

Test Plan: passed make check; also manually tested a few ldb subcommands

Reviewed By: pdillinger

Differential Revision: D21822188

Pulled By: gg814

fbshipit-source-id: 604cc0f0d0326a937ee25a32cdc2b512f9a3be6e
2020-06-03 18:57:49 -07:00
sdong 055b4d25f8 Make sure core components not depend on gtest (#6921)
Summary:
We recently removed the dependencies of core components on gtest. Add a Travis test to make sure it doesn't regress. Change cmake setting so that the gtest related components are only included when tests, benchmarks or stress tools are included in the build. Add this build setting in Travis to confirm it.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6921

Test Plan: See Travis passes

Reviewed By: ajkr

Differential Revision: D21863564

fbshipit-source-id: df26f50a8305a04ff19ffa8069a1857ecee10289
2020-06-03 18:22:14 -07:00
Stanislav Tkach b7c825d5cf Add (some) getters for options to the C API (#6925)
Summary:
Additionally I have extended the incomplete test added in the https://github.com/facebook/rocksdb/issues/6880.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6925

Reviewed By: ajkr

Differential Revision: D21869788

Pulled By: pdillinger

fbshipit-source-id: e9db80f259c57ca1bdcbc2c66cb938cb1ac26e48
2020-06-03 17:08:50 -07:00
sdong afa3518839 Revert "Update googletest from 1.8.1 to 1.10.0 (#6808)" (#6923)
Summary:
This reverts commit 8d87e9cea1.

Based on offline discussions, it's too early to upgrade to gtest 1.10, as it prevents some developers from using an older version of gtest to integrate to some other systems. Revert it for now.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6923

Reviewed By: pdillinger

Differential Revision: D21864799

fbshipit-source-id: d0726b1ff649fc911b9378f1763316200bd363fc
2020-06-03 15:55:03 -07:00
Hans Holmberg 0f85d163e6 Route GetTestDirectory to FileSystem in CompositeEnvWrappers (#6896)
Summary:
GetTestDirectory implies a file system operation (it creates the
default test directory if missing), so it should be routed to
the FileSystem rather than the Env.

Also remove the GetTestDirectory implementation in the PosixEnv,
since it overrides GetTestDirectory in CompositeEnv making it
impossible to override with a custom FileSystem.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6896

Reviewed By: cheng-chang

Differential Revision: D21868984

Pulled By: ajkr

fbshipit-source-id: e79bfef758d06dacef727c54b96abe62e78726fd
2020-06-03 14:57:46 -07:00
hfrt456 f005dac2d9 fix IsDirectory function in env_hdfs.cc (#6917)
Summary:
fix IsDirectory function for hdfsEnv
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6917

Reviewed By: cheng-chang

Differential Revision: D21865020

Pulled By: riversand963

fbshipit-source-id: ad69ed564d027b7bbdf4c693dd57cd02622fb3f8
2020-06-03 13:50:17 -07:00
Levi Tamasi 0b8c549b3f Mention the consistency check improvement in HISTORY.md (#6924)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6924

Reviewed By: cheng-chang

Differential Revision: D21865662

Pulled By: ltamasi

fbshipit-source-id: 83a01bcbb779cfba941154a36a9e735293a93211
2020-06-03 13:40:41 -07:00
Hao Chen ffe08ffcc2 correct level information in version_set.cc (#6920)
Summary:
fix these two issues https://github.com/facebook/rocksdb/issues/6912  and https://github.com/facebook/rocksdb/issues/6667
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6920

Reviewed By: cheng-chang

Differential Revision: D21864885

Pulled By: ajkr

fbshipit-source-id: 10e21fc1851b67a59d44358f59c64fa5523bd263
2020-06-03 12:27:13 -07:00
Anatoly Zhmur 22e5c513c2 Add zstd_max_train_bytes to c interop (#6796)
Summary:
Added setting of zstd_max_train_bytes compression option parameter to c interop.

rocksdb_options_set_bottommost_compression_options was using bool parameter and thus not exported, updated it to unsigned char and added to c.h as well.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6796

Reviewed By: cheng-chang

Differential Revision: D21611471

Pulled By: ajkr

fbshipit-source-id: caaaf153de934837ad9af283c7f8c025ff0b0cf5
2020-06-03 12:27:12 -07:00
mrambacher 0a17d95357 Add OptionTypeInfo::Vector to parse/serialize vectors (#6424)
Summary:
The OptionTypeInfo::Vector method allows a vector<T> to be converted to/from strings via the options.

The kVectorInt and kVectorCompressionType vectors were replaced with this methodology.

As part of this change, the NextToken method was added to the OptionTypeInfo.  This method was refactored from code within the StringToMap function.

Future types that could use this functionality include the EventListener vectors.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6424

Reviewed By: cheng-chang

Differential Revision: D21832368

Pulled By: zhichao-cao

fbshipit-source-id: e1ca766faff139d54e6e8407a9ec09ece6517439
2020-06-03 12:23:07 -07:00
Lucian Petrut 172adce767 Posix threads (#6865)
Summary:
Rocksdb is using the c++11 std::threads feature. The issue is that
MINGW only supports it when using Posix threads.

This change will allow rocksdb::port::WindowsThread to be replaced
with std::thread, which in turn will allow Rocksdb to be cross
compiled using MINGW.

At the same time, we'll have to use GetCurrentProcessId instead of _getpid.

Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6865

Reviewed By: cheng-chang

Differential Revision: D21864285

Pulled By: ajkr

fbshipit-source-id: 0982eed313e7d34d351b1364c1ccc722da473205
2020-06-03 12:01:57 -07:00
Peter Dillinger 43f8a9dcce Some fixes for gcc 4.8 and add to Travis (#6915)
Summary:
People keep breaking the gcc 4.8 compilation due to different
warnings for shadowing member functions with locals. Adding to Travis
to keep compatibility. (gcc 4.8 is default on CentOS 7.)
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6915

Test Plan: local and Travis

Reviewed By: siying

Differential Revision: D21842894

Pulled By: pdillinger

fbshipit-source-id: bdcd4385127ee5d1cc222d87e53fb3695c32a9d4
2020-06-03 11:39:25 -07:00
Levi Tamasi 78e291b17e Improve consistency checks in VersionBuilder (#6901)
Summary:
The patch cleans up the code and improves the consistency checks around
adding/deleting table files in `VersionBuilder`. Namely, it makes the checks
stricter and improves them in the following ways:
1) A table file can now only be deleted from the LSM tree using the level it
resides on. Earlier, there was some unnecessary wiggle room for
trivially moved files (they could be deleted using a lower level number than
the actual one).
2) A table file cannot be added to the tree if it is already present in the tree
on any level (not just the target level). The earlier code only had an assertion
(which is a no-op in release builds) that the newly added file is not already
present on the target level.
3) The above consistency checks around state transitions are now mandatory,
as opposed to the earlier `CheckConsistencyForDeletes`, which was a no-op
in release mode unless `force_consistency_checks` was set to `true`. The rationale
here is that assuming that the initial state is consistent, a valid transition leads to a
next state that is also consistent; however, an *invalid* transition offers no such
guarantee. Hence it makes sense to validate the transitions unconditionally,
and save `force_consistency_checks` for the paranoid checks that re-validate
the entire state.
4) The new checks build on the mechanism introduced in https://github.com/facebook/rocksdb/pull/6862,
which enables us to efficiently look up the location (level and position within level)
of files in a `Version` by file number. This makes the consistency checks much more
efficient than the earlier `CheckConsistencyForDeletes`, which essentially
performed a linear search.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6901

Test Plan:
Extended the unit tests and ran:

`make check`
`make whitebox_crash_test`

Reviewed By: ajkr

Differential Revision: D21822714

Pulled By: ltamasi

fbshipit-source-id: e2b29c8b6da1bf0f59004acc889e4870b2d18215
2020-06-03 11:24:55 -07:00
Peter Dillinger 9360776cb9 Fix handling of too-small filter partition size (#6905)
Summary:
Because ARM and some other platforms have a larger cache line
size, they have a larger minimum filter size, which causes recently
added PartitionedMultiGet test in db_bloom_filter_test to fail on those
platforms. The code would actually end up using larger partitions,
because keys_per_partition_ would be 0 and never == number of keys
added.

The code now attempts to get as close as possible to the small target
size, while fully utilizing that filter size, if the target partition
size is smaller than the minimum filter size.

Also updated the test to break more uniformly across platforms
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6905

Test Plan: updated test, tested on ARM

Reviewed By: anand1976

Differential Revision: D21840639

Pulled By: pdillinger

fbshipit-source-id: 11684b6d35f43d2e98b85ddb2c8dcfd59d670817
2020-06-03 10:43:01 -07:00
Zhichao Cao 2adb7e3768 Fix potential overflow of unsigned type in for loop (#6902)
Summary:
x.size() -1 or y - 1 can overflow to an extremely large value when x.size() pr y is 0 when they are unsigned type. The end condition of i in the for loop will be extremely large, potentially causes segment fault. Fix them.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6902

Test Plan: pass make asan_check

Reviewed By: ajkr

Differential Revision: D21843767

Pulled By: zhichao-cao

fbshipit-source-id: 5b8b88155ac5a93d86246d832e89905a783bb5a1
2020-06-02 15:05:07 -07:00
Zhichao Cao 556972e964 Replace Status with IOStatus in CopyFile and CreateFile (#6916)
Summary:
Replace Status with IOStatus in CopyFile and CreateFile.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6916

Test Plan: pass make asan_check

Reviewed By: cheng-chang

Differential Revision: D21843775

Pulled By: zhichao-cao

fbshipit-source-id: 524d4a0fcf47f0941b923da0346e0de71607f5f6
2020-06-02 15:05:06 -07:00
sdong bfc9737aca Remove gtest dependency in non-test code under utilities/cassandra (#6908)
Summary:
production code under utilities/cassandra depends on gtest.h. Remove them.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6908

Test Plan: Run all existing tests.

Reviewed By: ajkr

Differential Revision: D21842606

fbshipit-source-id: a098e0b49c9aeac51cc90a79562ad9897a36122c
2020-06-02 13:56:29 -07:00
Stanislav Tkach 38f988d3b4 Expose rocksdb_options_copy function to the C API (#6880)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6880

Reviewed By: ajkr

Differential Revision: D21842752

Pulled By: pdillinger

fbshipit-source-id: eda326f551ddd9cb397681544b9e9799ea614e52
2020-06-02 13:48:51 -07:00
Peter Dillinger 14eca6bf04 For ApproximateSizes, pro-rate table metadata size over data blocks (#6784)
Summary:
The implementation of GetApproximateSizes was inconsistent in
its treatment of the size of non-data blocks of SST files, sometimes
including and sometimes now. This was at its worst with large portion
of table file used by filters and querying a small range that crossed
a table boundary: the size estimate would include large filter size.

It's conceivable that someone might want only to know the size in terms
of data blocks, but I believe that's unlikely enough to ignore for now.
Similarly, there's no evidence the internal function AppoximateOffsetOf
is used for anything other than a one-sided ApproximateSize, so I intend
to refactor to remove redundancy in a follow-up commit.

So to fix this, GetApproximateSizes (and implementation details
ApproximateSize and ApproximateOffsetOf) now consistently include in
their returned sizes a portion of table file metadata (incl filters
and indexes) based on the size portion of the data blocks in range. In
other words, if a key range covers data blocks that are X% by size of all
the table's data blocks, returned approximate size is X% of the total
file size. It would technically be more accurate to attribute metadata
based on number of keys, but that's not computationally efficient with
data available and rarely a meaningful difference.

Also includes miscellaneous comment improvements / clarifications.

Also included is a new approximatesizerandom benchmark for db_bench.
No significant performance difference seen with this change, whether ~700 ops/sec with cache_index_and_filter_blocks and small cache or ~150k ops/sec without cache_index_and_filter_blocks.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6784

Test Plan:
Test added to DBTest.ApproximateSizesFilesWithErrorMargin.
Old code running new test...

    [ RUN      ] DBTest.ApproximateSizesFilesWithErrorMargin
    db/db_test.cc:1562: Failure
    Expected: (size) <= (11 * 100), actual: 9478 vs 1100

Other tests updated to reflect consistent accounting of metadata.

Reviewed By: siying

Differential Revision: D21334706

Pulled By: pdillinger

fbshipit-source-id: 6f86870e45213334fedbe9c73b4ebb1d8d611185
2020-06-02 12:30:23 -07:00
sdong 298b00a396 Reduce dependency on gtest dependency in release code (#6907)
Summary:
Release code now depends on gtest, indirectly through including "test_util/testharness.h". This creates multiple problems. One important reason is the definition of IGNORE_STATUS_IF_ERROR() in test_util/testharness.h. Move it to sync_point.h instead.
Note that utilities/cassandra/format.h still depends on "test_util/testharness.h". This will be resolved in a separate diff.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6907

Test Plan: Run all existing tests.

Reviewed By: ajkr

Differential Revision: D21829884

fbshipit-source-id: 9253c19ffde2936f3ae68998210f8e54f645a6e6
2020-06-02 12:11:24 -07:00
Adam Retter 8d87e9cea1 Update googletest from 1.8.1 to 1.10.0 (#6808)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6808

Reviewed By: anand1976

Differential Revision: D21483984

Pulled By: pdillinger

fbshipit-source-id: 70c5eff2bd54ddba469761d95e4cd4611fb8e598
2020-06-01 20:33:42 -07:00
anand76 66942e8158 Avoid unnecessary reads of uncompression dictionary in MultiGet (#6906)
Summary:
We may sometimes read the uncompression dictionary when its not
necessary, when we lookup a key in an SST file but the index indicates
the key is not present. This can happen with index_type 3.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6906

Test Plan: make check

Reviewed By: cheng-chang

Differential Revision: D21828944

Pulled By: anand1976

fbshipit-source-id: 7aef4f0a39548d0874eafefd2687006d2652f9bb
2020-06-01 19:43:37 -07:00
sdong 02f59ed669 Find the correct gcov (#6904)
Summary:
Right now in FB environment, wrong gcov is used. Fix it.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6904

Test Plan: "make coverage" and watch results.

Reviewed By: riversand963

Differential Revision: D21824291

fbshipit-source-id: 666011fd86c36adafa09ebd9eb97742f94fb90bb
2020-06-01 16:33:05 -07:00
Cheng Chang bcb9e41080 Explicitly free allocated buffer when status is not ok (#6903)
Summary:
Currently we rely on `BlockContents` to implicitly free the allocated scratch buffer, but when IO error happens, it doesn't make sense to construct the `BlockContents` which might be corrupted. In the stress test, we find that `assert(req.result.size() == block_size(handle));` fails because of potential IO errors.

In this PR, we explicitly free the scratch buffer on error without constructing `BlockContents`.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6903

Test Plan: watch stress test

Reviewed By: anand1976

Differential Revision: D21823869

Pulled By: cheng-chang

fbshipit-source-id: 5603fc80e9bf3f44a9d7250ddebd871afe1eb89f
2020-06-01 15:19:40 -07:00
zitan 038e02d8d9 Remove extraneous newline from ldb stderr (#6897)
Summary:
**Summary**
Remove the extraneous newline when using ldb tool. For example, the subcommand list_column_families will print an empty line to stderr even if there are no errors.

**Test plan**
Passed make check; manually tested a few ldb subcommands.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6897

Reviewed By: pdillinger

Differential Revision: D21819352

Pulled By: gg814

fbshipit-source-id: 5a16a6431bb96684fe97647f4d3ac5bf0ec7fc90
2020-06-01 12:15:36 -07:00
Peter Dillinger 0c56fc4d66 Allow missing "unversioned" python, as in CentOS 8 (#6883)
Summary:
RocksDB Makefile was assuming existence of 'python' command,
which is not present in CentOS 8. We avoid using 'python' if 'python3' is available.

Also added fancy logic to format-diff.sh to make clang-format-diff.py for Python2 work even with Python3 only (as some CentOS 8 FB machines come equipped)

Also, now use just 'python3' for PYTHON if not found so that an informative
"command not found" error will result rather than something weird.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6883

Test Plan: manually tried some variants, 'make check' on a fresh CentOS 8 machine without 'python' executable or Python2 but with clang-format-diff.py for Python2.

Reviewed By: gg814

Differential Revision: D21767029

Pulled By: pdillinger

fbshipit-source-id: 54761b376b140a3922407bdc462f3572f461d0e9
2020-05-29 11:29:23 -07:00
Andrew Kryczka c5abf78bca avoid IterKey::UpdateInternalKey() in BlockIter (#6843)
Summary:
`IterKey::UpdateInternalKey()` is an error-prone API as it's
incompatible with `IterKey::TrimAppend()`, which is used for
decoding delta-encoded internal keys. This PR stops using it in
`BlockIter`. Instead, it assigns global seqno in a separate `IterKey`'s
buffer when needed. The logic for safely getting a Slice with global
seqno properly assigned is encapsulated in `GlobalSeqnoAppliedKey`.
`BinarySeek()` is also migrated to use this API (previously it ignored
global seqno entirely).
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6843

Test Plan:
benchmark setup -- single file DBs, in-memory, no compression. "normal_db"
created by regular flush; "ingestion_db" created by ingesting a file. Both
DBs have same contents.

```
$ TEST_TMPDIR=/dev/shm/normal_db/ ./db_bench -benchmarks=fillrandom,compact -write_buffer_size=10485760000 -disable_auto_compactions=true -compression_type=none -num=1000000
$ ./ldb write_extern_sst ./tmp.sst --db=/dev/shm/ingestion_db/dbbench/ --compression_type=no --hex --create_if_missing < <(./sst_dump --command=scan --output_hex --file=/dev/shm/normal_db/dbbench/000007.sst | awk 'began {print "0x" substr($1, 2, length($1) - 2), "==>", "0x" $5} ; /^Sst file format: block-based/ {began=1}')
$ ./ldb ingest_extern_sst ./tmp.sst --db=/dev/shm/ingestion_db/dbbench/
```

benchmark run command:
```
TEST_TMPDIR=/dev/shm/$DB/ ./db_bench -benchmarks=seekrandom -seek_nexts=10 -use_existing_db=true -cache_index_and_filter_blocks=false -num=1000000 -cache_size=1048576000 -threads=1 -reads=40000000
```

results:

| DB | code | throughput |
|---|---|---|
| normal_db | master |  267.9 |
| normal_db   |    PR6843 | 254.2 (-5.1%) |
| ingestion_db |   master |  259.6 |
| ingestion_db |   PR6843 | 250.5 (-3.5%) |

Reviewed By: pdillinger

Differential Revision: D21562604

Pulled By: ajkr

fbshipit-source-id: 937596f836930515da8084d11755e1f247dcb264
2020-05-28 10:51:30 -07:00
Yanqin Jin 961c7590d6 Add timestamp to delete (#6253)
Summary:
Preliminary user-timestamp support for delete.

If ["a", ts=100] exists, you can delete it by calling `DB::Delete(write_options, key)` in which `write_options.timestamp` points to a `ts` higher than 100.

Implementation
A new ValueType, i.e. `kTypeDeletionWithTimestamp` is added for deletion marker with timestamp.
The reason for a separate `kTypeDeletionWithTimestamp`: RocksDB may drop tombstones (keys with kTypeDeletion) when compacting them to the bottom level. This is OK and useful if timestamp is disabled. When timestamp is enabled, should we still reuse `kTypeDeletion`, we may drop the tombstone with a more recent timestamp, causing deleted keys to re-appear.

Test plan (dev server)
```
make check
```
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6253

Reviewed By: ltamasi

Differential Revision: D20995328

Pulled By: riversand963

fbshipit-source-id: a9e5c22968ad76f98e3dc6ee0151265a3f0df619
2020-05-28 10:40:03 -07:00
Levi Tamasi e3f953a863 Make it possible to look up files by number in VersionStorageInfo (#6862)
Summary:
Does what it says on the can: the patch adds a hash map to `VersionStorageInfo`
that maps file numbers to file locations, i.e. (level, position in level) pairs. This
will enable stricter consistency checks in `VersionBuilder`. The patch also fixes
all the unit tests that used duplicate file numbers in a version (which would trigger
an assertion with the new code).
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6862

Test Plan:
`make check`
`make whitebox_crash_test`

Reviewed By: riversand963

Differential Revision: D21670446

Pulled By: ltamasi

fbshipit-source-id: 2eac249945cf33d8fb8597b26bfff5221e1a861a
2020-05-28 10:03:06 -07:00
Akanksha Mahajan bcefc59e9f Allow MultiGet users to limit cumulative value size (#6826)
Summary:
1. Add a value_size in read options which limits the cumulative value size of keys read in batches. Once the size exceeds read_options.value_size, all the remaining keys are returned with status Abort without further fetching any key.
2. Add a unit test case MultiGetBatchedValueSizeSimple the reads keys from memory and sst files.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6826

Test Plan:
1. make check -j64
	   2. Add a new unit test case

Reviewed By: anand1976

Differential Revision: D21471483

Pulled By: akankshamahajan15

fbshipit-source-id: dea51b8e76d5d1df38ece8cdb29933b1d798b900
2020-05-27 13:07:14 -07:00
Adam Retter 9060e6fa79 Add newer WBWI::NewIteratorWithBase functions to RocksJava (#6872)
Summary:
Exposes the `ReadOptions` arguments to `NewIteratorWithBase`.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6872

Reviewed By: ajkr

Differential Revision: D21725867

Pulled By: pdillinger

fbshipit-source-id: 4079ba590cc13ba7a6244ed91439d89c40a543b6
2020-05-27 11:59:12 -07:00
Cheng Chang 82a82c76e7 Fix potential memory leak of scratch buffer (#6879)
Summary:
If `req.scratch` is an internally allocated buffer, but `raw_block_contents` is not constructed to own `req.scratch`, then `req.scratch` will be leaked.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6879

Test Plan: make asan_check

Reviewed By: anand1976

Differential Revision: D21728498

Pulled By: cheng-chang

fbshipit-source-id: 8fc6a4f2543918c565ddc16ecfad1807eb9a42cf
2020-05-26 15:29:04 -07:00
Kefu Chai bd68bfb41b cmake: link env_librados_test against rados (#6855)
Summary:
otherwise we have FTBFS like:
2020-05-18T15:12:06.400 INFO:tasks.workunit.client.0.smithi032.stdout:[100%] Linking CXX executable env_librados_test
2020-05-18T15:12:06.620 INFO:tasks.workunit.client.0.smithi032.stderr:/usr/bin/ld: CMakeFiles/rocksdb_env_librados_test.dir/utilities/env_librados_test.cc.o: undefined reference to symbol
'_ZN8librados7v14_2_05Rados4initEPKc@LIBRADOS_14.2.0'
2020-05-18T15:12:06.620 INFO:tasks.workunit.client.0.smithi032.stderr:/usr/bin/ld: /lib/librados.so.2: error adding symbols: DSO missing from command line
2020-05-18T15:12:06.620 INFO:tasks.workunit.client.0.smithi032.stderr:collect2: error: ld returned 1 exit status

this addresses the regression introduced by 07204837ce,
which hides the symbols exposed by `${THIRDPARTY_LIBS}` from
consumers of librocksdb

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6855

Reviewed By: riversand963

Differential Revision: D21621904

fbshipit-source-id: 7022ba4dc0003504401fce6f06547e4d74a32ac0
2020-05-25 22:53:00 -07:00
Andrew Kryczka b464a85e33 fix transaction rollback in db_stress TestMultiGet (#6873)
Summary:
There were further uses of `txn` after `RollbackTxn(txn)` leading to
stress test errors. Moved the rollback to the end of the function.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6873

Test Plan:
found a command from the crash test that previously failed immediately under TSAN; verified now it succeeds.
```
./db_stress --acquire_snapshot_one_in=10000 --allow_concurrent_memtable_write=1 --avoid_flush_during_recovery=0 --avoid_unnecessary_blocking_io=1 --block_size=16384 --bloom_bits=222.913637674 --bottommost_compression_type=none --cache_index_and_filter_blocks=1 --cache_size=1048576 --checkpoint_one_in=0 --checksum_type=kCRC32c --clear_column_family_one_in=0 --compact_files_one_in=1000000 --compact_range_one_in=1000000 --compaction_style=1 --compaction_ttl=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=1048576 --delpercent=5 --delrangepercent=0 --destroy_db_initially=0 --disable_wal=0 --enable_pipelined_write=0 --flush_one_in=1000000 --format_version=5 --get_current_wal_file_one_in=0 --get_live_files_one_in=1000000 --get_sorted_wal_files_one_in=0 --index_block_restart_interval=12 --index_type=2 --key_len_percent_dist=1,30,69 --level_compaction_dynamic_level_bytes=True --log2_keys_per_lock=22 --long_running_snapshots=0 --max_background_compactions=20 --max_bytes_for_level_base=10485760 --max_key=1000000 --max_key_len=3 --max_manifest_file_size=1073741824 --max_write_batch_group_size_bytes=1048576 --max_write_buffer_number=3 --memtablerep=skip_list --mmap_read=1 --mock_direct_io=False --nooverwritepercent=1 --num_levels=1 --open_files=100 --ops_per_thread=200000 --partition_filters=1 --pause_background_one_in=1000000 --periodic_compaction_seconds=0 --prefixpercent=5 --progress_reports=0 --read_fault_one_in=0 --readpercent=45 --recycle_log_file_num=0 --reopen=20 --snapshot_hold_ops=100000 --subcompactions=4 --sync=0 --sync_fault_injection=False --target_file_size_base=2097152 --target_file_size_multiplier=2 --test_batches_snapshots=0 --txn_write_policy=1 --unordered_write=1 --use_block_based_filter=0 --use_direct_io_for_flush_and_compaction=0 --use_direct_reads=0 --use_full_merge_v1=1 --use_merge=1 --use_multiget=1 --use_txn=1 --verify_checksum=1 --verify_checksum_one_in=1000000 --verify_db_one_in=100000 --write_buffer_size=4194304 --write_dbid_to_manifest=0 --writepercent=35
```

Reviewed By: pdillinger

Differential Revision: D21708338

Pulled By: ajkr

fbshipit-source-id: dcf55cddee0a14f429a75e7a8a505acf8025f2b1
2020-05-24 15:27:24 -07:00
Andrew Kryczka b559dba817 pin image version in circle CI builds (#6876)
Summary:
somehow the windows-server-2019-vs2019 image changed in a way that made
VS 14 2015 the default. This caused an error when we specify VS 16 2019
as the cmake generator. I could not figure out the right arguments/env
vars to get the latest VS working so pinned the image to the previous
version instead.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6876

Reviewed By: pdillinger

Differential Revision: D21709679

Pulled By: ajkr

fbshipit-source-id: 2d16819ad239b4611fa199547744e1c101dc9da0
2020-05-23 21:23:27 -07:00
Peter Dillinger 803a517b48 Misc things for ASSERT_STATUS_CHECKED, also gcc 4.8.5 (#6871)
Summary:
* Print stack trace on status checked failure
* Make folly_synchronization_distributed_mutex_test a parallel test
* Disable ldb_test.py and rocksdb_dump_test.sh with
  ASSERT_STATUS_CHECKED (broken)
* Fix shadow warning in random_access_file_reader.h reported by gcc
  4.8.5 (ROCKSDB_NO_FBCODE), also https://github.com/facebook/rocksdb/issues/6866
* Work around compiler bug on max_align_t for gcc < 4.9
* Remove an apparently wrong comment in status.h
* Use check_some in Travis config (for proper diagnostic output)
* Fix ignored Status in loop in options_helper.cc
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6871

Test Plan: manual, CI

Reviewed By: ajkr

Differential Revision: D21706619

Pulled By: pdillinger

fbshipit-source-id: daf6364173d6689904eb394461a69a11f5bee2cb
2020-05-23 06:53:37 -07:00
Marek Kurdej bcd32560dd Fix warning -Wextra-semi. NFC. (#6869)
Summary:
Minor fix.

CLA signed.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6869

Reviewed By: ajkr

Differential Revision: D21704001

Pulled By: pdillinger

fbshipit-source-id: 57fd08114f3234f51f34758e25e708cc70962582
2020-05-22 11:20:13 -07:00
Peter Dillinger 35a25a3fb9 Fix/expand ASSERT_STATUS_CHECKED build, add to Travis (#6870)
Summary:
Fixed some option handling code that recently broke the
ASSERT_STATUS_CHECKED build for options_test.

Added all other existing tests that pass under ASSERT_STATUS_CHECKED to
the whitelist.

Added a Travis configuration to run all whitelisted tests with
ASSERT_STATUS_CHECKED. (Someday we might enable this check by default in
debug builds.)
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6870

Test Plan: ASSERT_STATUS_CHECKED=1 make check, Travis

Reviewed By: ajkr

Differential Revision: D21704374

Pulled By: pdillinger

fbshipit-source-id: 15daef98136a19d7a6843fa0c9ec08738c2ac693
2020-05-22 11:17:29 -07:00
mrambacher 826295a5e9 Change autovector to have a reserved size in LITE mode (#6868)
Summary:
Previously in LITE mode, an autovector did not have a reserved size. When
elements were added to the vector, the underlying array could be reallocated.

There was a set of code that never expands the autovector and was doing &autovector::back().  When the vector is resized, the old addresses may become invalid, causing a later exception to be thrown.

By reserving space in the autovector up front, this problem is eliminated for those uses where the vector will never exceed the initial size.

the resize happens, these pointers become invalid, leading to SEGV or other exceptions.

This change allows the autovector to be fully populated before we take the address of any of its elements, thereby elminating the potential for a resize.

There is comparable code to this change in Version::MultiGet for dealing with the context objects.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6868

Reviewed By: ajkr

Differential Revision: D21693505

Pulled By: cheng-chang

fbshipit-source-id: e71d516b15e08f202593cb80f2a42f048fc95768
2020-05-21 14:48:10 -07:00
Andrew Kryczka 292bcf6227 skip direct I/O tests in rocksdb lite (#6867)
Summary:
Fix a couple places where direct I/O was used even though it is
unsupported in lite builds.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6867

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

Reviewed By: pdillinger

Differential Revision: D21689185

Pulled By: ajkr

fbshipit-source-id: 3eaa3abf69cd7d0bcaabbcad3bb5a26fb8dd7301
2020-05-21 13:57:17 -07:00
mrambacher 38be686160 Add Struct Type to OptionsTypeInfo (#6425)
Summary:
Added code for generically handing structs to OptionTypeInfo.  A struct is a collection of variables handled by their own map of OptionTypeInfos.  Examples of structs include Compaction and Cache options.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6425

Reviewed By: siying

Differential Revision: D21668789

Pulled By: zhichao-cao

fbshipit-source-id: 064b110de39dadf82361ed4663f7ac1a535b0b07
2020-05-21 10:58:39 -07:00