Commit Graph

9568 Commits

Author SHA1 Message Date
Peter Dillinger
327ddb7d47 Travis: fail fast on install failures (#7239)
Summary:
A recent build continued with confusing results after failing
to "snap install cmake" so ensure failures in installs are fatal. Also
upgrade snapd before "snap install" to hopefully avoid this error:

    error: cannot perform the following tasks:
    - Mount snap "cmake" (513) (snap "cmake" assumes unsupported features: command-chain (try to update snapd and refresh the core snap))

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

Test Plan: watch Travis

Reviewed By: akankshamahajan15

Differential Revision: D23244110

Pulled By: pdillinger

fbshipit-source-id: 33dbf145f6999d0b90576cdfde484f15c5d1ac19
2020-08-20 10:39:10 -07:00
Jay Zhuang
3e422ce0ca Fix a timer_test deadlock (#7277)
Summary:
There's a potential deadlock caused by MockTimeEnv time value get to a large number, which causes TimedWait() wait forever. The test misuses the microseconds as seconds, making it more likely to happen.

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

Reviewed By: pdillinger

Differential Revision: D23183873

Pulled By: jay-zhuang

fbshipit-source-id: 6fc38ebd40b4125a99551204b271f91a27e70086
2020-08-20 08:43:13 -07:00
Will Angenent
2040bb545b Fixed missing space in DeleteScheduler::BackgroundEmptyTrash log message (#7286)
Summary:
Example of a faulty log message:
Rate limiting is enabled with penalty 18203625after deleting file ...

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

Reviewed By: David132639

Differential Revision: D23215981

Pulled By: freewilll

fbshipit-source-id: 8bdbbffea9f2942cc7a652f315a560d61c0f1068
2020-08-20 02:00:06 -07:00
Jay Zhuang
ac7dcfda10 Add missing ComputeCompactionScore() for a new universal manual compaction (#7281)
Summary:
Seems it's only causing assert failure during compaction pick, but in production code, the problematic compactions are excluded at a later step.

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

Reviewed By: akankshamahajan15

Differential Revision: D23228000

Pulled By: jay-zhuang

fbshipit-source-id: 2e4055aeebe0f5a2b07e299e0a2d51a1ad2e216d
2020-08-19 17:42:08 -07:00
Levi Tamasi
b9bb59d49d Add initial set of options for integrated blob write path (#7280)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7280

Test Plan: `make check`

Reviewed By: riversand963

Differential Revision: D23195192

Pulled By: ltamasi

fbshipit-source-id: 743b382de391963e62ba86119e9fbd0233ea3b3a
2020-08-18 18:32:37 -07:00
Akanksha Mahajan
cc24ac14eb Store FSSequentialFilePtr object in SequenceFileReader (#7190)
Summary:
This diff contains following changes:
    1. Replace `FSSequentialFile` pointer with `FSSequentialFilePtr` object that wraps `FSSequentialFile` pointer in `SequenceFileReader`.

Objective: If tracing is enabled, `FSSequentialFilePtr` returns `FSSequentialFileTracingWrapper` pointer that includes all necessary information in `IORecord` and calls underlying FileSystem and invokes `IOTracer` to dump that record in a binary file. If tracing is disabled then, underlying `FileSystem` pointer is returned directly. `FSSequentialFilePtr` wrapper class is added to bypass the `FSSequentialFileTracingWrapper` when tracing is disabled.

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

Test Plan:
make check -j64
          COMPILE_WITH_TSAN=1 make check -j64

Reviewed By: anand1976

Differential Revision: D23059616

Pulled By: akankshamahajan15

fbshipit-source-id: 1564b94dd1297cd0fbfe2ed5c9cc3e20f7395301
2020-08-18 16:20:54 -07:00
Andrew Kryczka
e6e2f3699c fix doc about kTolerateCorruptedTailRecords recovery (#7270)
Summary:
- Made it clear only one record in the tail is allowed to have a problem
- Added detail about the valid use case instead of calling it legacy behavior

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

Reviewed By: riversand963

Differential Revision: D23169075

Pulled By: ajkr

fbshipit-source-id: 2a4b45aa8641f17efa104523fbad765012a98fb0
2020-08-18 09:52:25 -07:00
Peter Dillinger
7d0ecab570 Fix some flaky tests in BackupableDBTest with intentional flushing (#7273)
Summary:
Some tests like BackupableDBTest.FileCollision and
ShareTableFilesWithChecksumsNewNaming are intermittently failing,
probably due to unpredictable flushing with FillDB. This change
should fix the failures seen and help to prevent similar flakiness in
future tests in the file.

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

Test Plan: make check, and with valgrind

Reviewed By: siying

Differential Revision: D23176947

Pulled By: pdillinger

fbshipit-source-id: 654b73a64db475f2b9b065ed53a889a8b9083c59
2020-08-17 22:07:17 -07:00
Jay Zhuang
c073b7faef db_bench should be linked with thirdparty libs (#7264)
Summary:
`db_bench` is not linked with thirdparty libs in cmake, even `-DWITH_*`
is specified.

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

Test Plan:
`$ mkdir build; cd build; cmake .. -DWITH_SNAPPY=1; make db_bench; ./db_bench`
`$ cmake .. -DWITH_SNAPPY=1 -DWITH_LZ4; make db_bench; ./db_bench -compression_type=lz4`

Reviewed By: zhichao-cao

Differential Revision: D23165077

Pulled By: jay-zhuang

fbshipit-source-id: 9c6fead31c41664a5c75ecd6469f47402fcb7d62
2020-08-17 18:54:14 -07:00
sdong
b194c21bba Whole DBTest to skip fsync (#7274)
Summary:
After https://github.com/facebook/rocksdb/pull/7036, we still see extra DBTest that can timeout when running 10 or 20 in parallel. Expand skip-fsync mode in whole DBTest. Still preserve other tests from doing this mode to be conservative.

This commit reinstates https://github.com/facebook/rocksdb/issues/7049, whose un-revert was lost in an automatic
infrastructure mis-merge.

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

Test Plan: Run all existing files.

Reviewed By: pdillinger

Differential Revision: D23177444

fbshipit-source-id: 1f61690b2ac6333c3b2c87176fef6b2cba086b33
2020-08-17 18:42:25 -07:00
Andrew Kryczka
5d5ff82408 Disable recycle_log_file_num with kTolerateCorruptedTailRecords (#7271)
Summary:
The two features are naturally incompatible. WAL recycling expects the recovery to succeed upon encountering a corrupt record at the point where new data ends and recycled data remains at the tail. However, `WALRecoveryMode::kTolerateCorruptedTailRecords` must fail upon encountering any such corrupt record, as it cannot differentiate between this and a real corruption, which would cause committed updates to be truncated.

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

Reviewed By: riversand963

Differential Revision: D23169923

Pulled By: ajkr

fbshipit-source-id: 2cf8a3bcd2c9a0ecb0055a84725047a10fd4db50
2020-08-17 18:21:10 -07:00
Yanqin Jin
92593d511a Add a new EntryType for deletion with timestamp (#7195)
Summary:
Add `kEntryDeleteWithTimestamp` to `EntryType` which is a public API.

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

Test Plan: make check

Reviewed By: ajkr

Differential Revision: D22914704

Pulled By: riversand963

fbshipit-source-id: 886f73c6b70c527cad1c8fc9fc8d3afe60e1ea39
2020-08-17 16:26:06 -07:00
Levi Tamasi
9b083cb11c Build blob file reader/writer classes in LITE mode as well (#7272)
Summary:
The patch makes sure that the functionality required for the new integrated
BlobDB implementation (most importantly, the classes related to reading and
writing blob files) is also built in LITE mode by removing the corresponding
`#ifndef`s.

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

Test Plan: Ran `make check` in both regular and LITE mode.

Reviewed By: zhichao-cao

Differential Revision: D23173280

Pulled By: ltamasi

fbshipit-source-id: 1596bd1a76409a8a6d83d8f1dbfe08bfdea7ffe6
2020-08-17 15:19:05 -07:00
sdong
1760637539 CompactRange() refit level should confirm destination level is not empty (#7261)
Summary:
There is potential data race related CompactRange() with level refitting. After the compaction step and refitting step, some automatic compaction could put data to the destination level and cause the DB to be corrupted. Fix the bug by checking the target level to be empty.

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

Test Plan: Add a unit test, which would fail with "Corruption: L1 have overlapping ranges '666F6F' seq:6, type:1 vs. '626172' seq:2, type:1", and now it succeeds.

Reviewed By: ajkr

Differential Revision: D23142269

fbshipit-source-id: 28bc14d5ac934c192260b23a4ce3f10a95e3ee91
2020-08-17 14:21:53 -07:00
Zitan Chen
500eeb6fd3 Re-enable param tests for backup engine (#7260)
Summary:
The param tests did not take any effect previously. This PR re-enables it.

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

Test Plan: Some manual tests and `./backupable_db_test`.

Reviewed By: siying

Differential Revision: D23140902

Pulled By: pdillinger

fbshipit-source-id: cd62b11b926affed25127d9074fa97a1c7f748c4
2020-08-17 13:59:21 -07:00
matthewvon
2ad88ceae9 Populate cf_id member of CompactionJobInfo for OnCompactionBegin (#6938)
Summary:
Looks like somebody simply missed initializing a member variable. The column family ID, cf_id, is not set during OnCompactionBegin. But it is set properly in the next function for OnCompactionCompleted. Need this cf_id for tracking progress of a Stardog optimize since there may be multiple compactions required for a given column family.

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

Reviewed By: siying

Differential Revision: D23153235

Pulled By: ajkr

fbshipit-source-id: 932938de3a4ebbc7ac89702f655583862587d251
2020-08-17 11:57:47 -07:00
Hans Holmberg
2a0d3c7054 Add a file system parameter: --fs_uri to db_stress and db_bench (#6878)
Summary:
This pull request adds the parameter --fs_uri to db_bench and db_stress, creating a composite env combining the default env with a specified registered rocksdb file system.

This makes it easier to develop and test new RocksDB FileSystems.

The pull request also registers the posix file system for testing purposes.

Examples:
```
$./db_bench --fs_uri=posix:// --benchmarks=fillseq

$./db_stress --fs_uri=zenfs://nullb1
```

zenfs is a RocksDB FileSystem I'm developing to add support for zoned block devices, and in that case the zoned block device is specified in the uri (a zoned null block device in the above example).

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

Reviewed By: siying

Differential Revision: D23023063

Pulled By: ajkr

fbshipit-source-id: 8b3fe7193ce45e683043b021779b7a4d547af247
2020-08-17 11:55:24 -07:00
John Goerzen
59ebab654e Generate and install a pkg-config file (#7244)
Summary:
pkg-config files are quite useful for communicating to users of a
library how to compile against them. This commit generates and installs
a pkg-config file that can be used for both static and dynamic builds
against the RocksDB library. This should make life easier for developers
of client programs, language bindings, etc.

Example usage:

```
g++ `pkg-config --cflags rocksdb` -o simple_example simple_example.cc `pkg-config --libs rocksdb`

g++ `pkg-config --cflags --static rocksdb` -static \
   -o simple_example simple_example.cc `pkg-config --libs --static rocksdb`
```

The commit also adds the generated file to .gitignore, to the uninstall
target, and to clean.

No additional dependencies are added to RocksDB itself, and this does
not make RocksDB use pkg-config as part of its build process.

Resolves https://github.com/facebook/rocksdb/issues/4452

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

Reviewed By: siying

Differential Revision: D23146153

Pulled By: ajkr

fbshipit-source-id: 3045aa650d68bd5ac42d40ed709570e9584ef004
2020-08-17 11:53:11 -07:00
Jay Zhuang
69760b4d05 Introduce a global StatsDumpScheduler for stats dumping (#7223)
Summary:
Have a global StatsDumpScheduler for all DB instance stats dumping, including `DumpStats()` and `PersistStats()`. Before this, there're 2 dedicate threads for every DB instance, one for DumpStats() one for PersistStats(), which could create lots of threads if there're hundreds DB instances.

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

Reviewed By: riversand963

Differential Revision: D23056737

Pulled By: jay-zhuang

fbshipit-source-id: 0faa2311142a73433ebb3317361db7cbf43faeba
2020-08-14 20:12:44 -07:00
Yanqin Jin
d758273ceb Get() with timestamp should respect snapshot (#7227)
Summary:
If user-defined timestamp is enabled, current implementation can expose
newer data to queries even if an older sequence number is specified via
read_options.snapshot. This PR makes Get() respect sequence-number-based
snapshot.

Solution is simple. Besides using <ukey, ts, seq> to search the index for the key,
we also verify that the candidate result's seq is smaller than or equal to seq. This
requires passing a seq via `GetContext`, which results in the majority of code
change caused by this PR.

Also added a few unit tests to demonstrate standard visibility during point lookup
and range scan when timestamp and snapshot are both present.

Test plan (devserver):
```
make check
$./db_bench --benchmarks=fillseq,readrandom -cache_size=$[64*1024*1024]
```
Result
this PR: readrandom   :       4.827 micros/op 207180 ops/sec;   22.9 MB/s (1000000 of 1000000 found)
master:  readrandom   :       4.936 micros/op 202610 ops/sec;   22.4 MB/s (1000000 of 1000000 found)

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

Reviewed By: ltamasi

Differential Revision: D23015242

Pulled By: riversand963

fbshipit-source-id: ea7b85a728654553ba357d2e6a207b5e40f7376a
2020-08-14 19:20:58 -07:00
Daniel Smith
510c66fb75 Typo: s/entires/entries/ (#7248)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7248

Reviewed By: ajkr

Differential Revision: D23108368

Pulled By: jay-zhuang

fbshipit-source-id: 41765334b0bae26bd74a4d9250b9b98ea7ac6c3f
2020-08-14 13:37:23 -07:00
Zitan Chen
15245e9018 Fix flaky BackupableDBTest.CustomChecksumTransition (#7254)
Summary:
The flaky test in the title is caused by two problems. First, there is a bug in the BackupEngine that results in skipping computing the default crc32 checksum when `share_table_files` is enabled and the table is already backed up. Second, when `RestoreDBFromBackup` fails and the backup was being restored to the DB directory, it is likely that `RestoreDBFromBackup` has cleaned up the DB directory before it fails, and therefore, files in old backups may collide with files to be backed up if `share_files_with_checksum` is not enabled.

New tests that cover the above problems are added.

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

Test Plan: `./backupable_db_test`

Reviewed By: pdillinger

Differential Revision: D23118715

Pulled By: gg814

fbshipit-source-id: 7be8de912808944be59e93d602c7431a54c079eb
2020-08-14 13:34:15 -07:00
Andrew Kryczka
a1aa3f8385 Disable manual compaction during ReFitLevel() (#7250)
Summary:
Manual compaction with `CompactRangeOptions::change_levels` set could
refit to a level targeted by another manual compaction. If
force_consistency_checks were disabled, it could be possible for
overlapping files to be written at that target level.

This PR prevents the possibility by calling `DisableManualCompaction()`
prior to `ReFitLevel()`. It also improves the manual compaction disabling
mechanism to wait for pending manual compactions to complete before
returning, and support disabling from multiple threads.

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

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

Test Plan:
crash test command that repro'd the bug reliably:

```
$ TEST_TMPDIR=/dev/shm python tools/db_crashtest.py blackbox --simple -target_file_size_base=524288 -write_buffer_size=1048576 -clear_column_family_one_in=0 -reopen=0 -max_key=10000000 -column_families=1 -max_background_compactions=8 -compact_range_one_in=100000 -compression_type=none -compaction_style=1 -num_levels=5 -universal_min_merge_width=4 -universal_max_merge_width=8 -level0_file_num_compaction_trigger=12 -rate_limiter_bytes_per_sec=1048576000 -universal_max_size_amplification_percent=100 --duration=3600 --interval=60 --use_direct_io_for_flush_and_compaction=0 --use_direct_reads=0 --enable_compaction_filter=0
```

Reviewed By: ltamasi

Differential Revision: D23090800

Pulled By: ajkr

fbshipit-source-id: afcbcd51b42ce76789fdb907d8b9ada790709c13
2020-08-14 11:29:52 -07:00
Adam Retter
e503f5e0a0 RocksJava should not limit valid format_version (#7242)
Summary:
Previously RocksJava limited the format_version to 4. However, the C++ API is now at 5, and this will likely increase again in future. The Java API now allows any positive integer, and an exception is raised from JNI if the format_version is out-of-bounds.

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

Reviewed By: cheng-chang

Differential Revision: D23077941

Pulled By: pdillinger

fbshipit-source-id: ee69f7203448acddc41c6d86b470ed987d3d366d
2020-08-13 20:43:28 -07:00
sdong
4b0a509a91 Still use platform007 for gcc (#7253)
Summary:
We see some hosts failed to build platform009 with gcc. Revert the default to be platform007 if USE_CLANG is not specified.

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

Test Plan: Build with both of USE_CLANG=1 set and not set and observe it builds successfully, and see the tool chain used.

Reviewed By: jay-zhuang

Differential Revision: D23110550

fbshipit-source-id: 25cb47923f7174b24debdad0cc8d90b07c4d5d09
2020-08-13 14:49:34 -07:00
sdong
e7358da9a2 Upgrade tool chain (#7251)
Summary:
Upgrade tool chain to the latest. It is done mostly manually as build_tools/build_detect_platform fails to update many of them.

Try to fix a new clang analyze warning with the new tool chain.

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

Test Plan: "make all", "USE_CLANG=1 make all"

Reviewed By: riversand963

Differential Revision: D23091090

fbshipit-source-id: 732e5a30137837431438f85f36296406b641f975
2020-08-12 19:30:00 -07:00
Levi Tamasi
9d6f48ec1d Clean up CompressBlock/CompressBlockInternal a bit (#7249)
Summary:
The patch cleans up and refactors `CompressBlock` and `CompressBlockInternal` a bit.
In particular, it does the following:
* It renames `CompressBlockInternal` to `CompressData` and moves it to `util/compression.h`,
where other general compression-related utilities are located. This will facilitate reuse in the
BlobDB write path.
* The signature of the method is changed so it now takes `compression_format_version`
(similarly to the compression library specific methods) instead of `format_version` (which is
specific to the block based table).
* `GetCompressionFormatForVersion` no longer takes `compression_type` as a parameter.
This parameter was only used in a (not entirely up-to-date) assertion; also, removing it
eliminates the need to ensure this precondition holds at all call sites.
* Does some minor cleanup in `CompressBlock`, for instance, it is now possible to pass
only one of `sampled_output_fast` and `sampled_output_slow`.

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

Test Plan: `make check`

Reviewed By: riversand963

Differential Revision: D23087278

Pulled By: ltamasi

fbshipit-source-id: e6316e45baed8b4e7de7c1780c90501c2a3439b3
2020-08-12 18:25:48 -07:00
Akanksha Mahajan
1f9f630b27 Store FileSystemPtr object that contains FileSystem ptr (#7180)
Summary:
As part of the IOTracing project, this PR
    1. Caches "FileSystemPtr" object(wrapper class that returns file system pointer based on tracing enabled) instead of "FileSystem" pointer.
    2. FileSystemPtr object is created using FileSystem pointer and IOTracer
    pointer.
    3. IOTracer shared_ptr is created in DBImpl and it is passed to different classes through constructor.
    4. When tracing is enabled through DB::StartIOTrace, FileSystemPtr
    returns FileSystemTracingWrapper pointer for tracing purpose and when
    it is disabled underlying FileSystem pointer is returned.

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

Test Plan:
make check -j64
                COMPILE_WITH_TSAN=1 make check -j64

Reviewed By: anand1976

Differential Revision: D22987117

Pulled By: akankshamahajan15

fbshipit-source-id: 6073617e4c2d5bc363914f3a1f55ae3b0a58fbf1
2020-08-12 17:31:23 -07:00
Arkady Dyakonov
2bc63e3aba Fix Java test for uint64add merge operator (#7243)
Summary:
The PR fixes a Java test for Merge operator `uint64add`.

The current implementation uses wrong byte order for long serialization, but fails to catch this error because the merge sum is lower than `256`.

The PR makes this test case more representative (i.e. it fails with wrong byte order) and changes the byte order to little endian.

Some background: RocksDB uses LittleEndian byte order for integer serialization across all platforms. `MergeTest` uses `ByteBuffer` that defaults to BigEndian byte order.

This test case might probably be used as a sample of `MergeOperator` usage in Java.

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

Reviewed By: ajkr

Differential Revision: D23079593

Pulled By: pdillinger

fbshipit-source-id: 82e8e166901d66733e96a0116f88d0ec4761ddf1
2020-08-12 14:36:08 -07:00
Zitan Chen
b578ca2e4d BackupEngine supports custom file checksums (#7085)
Summary:
A new option `std::shared_ptr<FileChecksumGenFactory> backup_checksum_gen_factory` is added to `BackupableDBOptions`. This allows custom checksum functions to be used for creating, verifying, or restoring backups.

Tests are added.

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

Test Plan: Passed make check

Reviewed By: pdillinger

Differential Revision: D22390756

Pulled By: gg814

fbshipit-source-id: 3b7756ca444c2129844536b91c3ca09f53b6248f
2020-08-12 13:31:09 -07:00
Yanqin Jin
76609cd38a Fix potential memory leak (#7245)
Summary:
```
int* value = new int;
ASSERT_NE(nullptr, value);
```
`ASSERT_NE` can expand the expression such that a memory leak is
reported by clang analyzer.
We can remove this ASSERT_NE since we can assume the memory allocation
must succeed. Otherwise a bad alloc exception will be thrown and the
process will be killed anyway.

Test plan (dev server):
```
USE_CLANG=1 make analyze
```

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

Reviewed By: jay-zhuang

Differential Revision: D23079641

Pulled By: riversand963

fbshipit-source-id: a6739a903f90f8715f6f1ef3e5c8a329245b8e78
2020-08-12 12:03:22 -07:00
Levi Tamasi
378bc94d7e Update github-pages to v207 (#7235)
Summary:
The patch updates github-pages to the latest version. Dependencies were
updated using `bundle update`. Also, the deprecated option `gems` is replaced
with `plugins` in the Jekyll config.

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

Test Plan: `bundle exec jekyll serve`

Reviewed By: pdillinger

Differential Revision: D23034419

Pulled By: ltamasi

fbshipit-source-id: a3f6df1c33281bdfd33aa61c6dc92162d9b7f079
2020-08-12 09:26:24 -07:00
Yanqin Jin
f15414b656 Timer should run scheduled function without mutex (#7228)
Summary:
Timer (defined in timer.h) schedules and runs user-specified fuctions
regularly. Current implementation holds the mutex while running user
function, which will lead to contention and waiting.
To fix, Timer::Run releases mutex before running user function, and
re-acquires it afterwards.
This fix will impact how we can cancel a task. If the task is running,
it is not holding the mutex. The thread calling Cancel() should wait
until the current task finishes.

Test Plan (devserver):
make check
COMPILE_WITH_ASAN=1 make check

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

Reviewed By: jay-zhuang

Differential Revision: D23065487

Pulled By: riversand963

fbshipit-source-id: 07cb59741f506d3eb875c8ab90f73437568d3724
2020-08-11 22:37:39 -07:00
Yanqin Jin
e9daedec84 Add CircleCI for tests on non-shm (#7229)
Summary:
As title.

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

Test Plan: Watch for CircleCI results.

Reviewed By: siying

Differential Revision: D23023943

Pulled By: riversand963

fbshipit-source-id: 41a989a3ffdfd2decd309185e3b963e810419577
2020-08-11 18:30:47 -07:00
Peter Dillinger
6ac1d25fd0 Fix+clean up handling of mock sleeps (#7101)
Summary:
We have a number of tests hanging on MacOS and windows due to
mishandling of code for mock sleeps. In addition, the code was in
terrible shape because the same variable (addon_time_) would sometimes
refer to microseconds and sometimes to seconds. One test even assumed it
was nanoseconds but was written to pass anyway.

This has been cleaned up so that DB tests generally use a SpecialEnv
function to mock sleep, for either some number of microseconds or seconds
depending on the function called. But to call one of these, the test must first
call SetMockSleep (precondition enforced with assertion), which also turns
sleeps in RocksDB into mock sleeps. To also removes accounting for actual
clock time, call SetTimeElapseOnlySleepOnReopen, which implies
SetMockSleep (on DB re-open). This latter setting only works by applying
on DB re-open, otherwise havoc can ensue if Env goes back in time with
DB open.

More specifics:

Removed some unused test classes, and updated comments on the general
problem.

Fixed DBSSTTest.GetTotalSstFilesSize using a sync point callback instead
of mock time. For this we have the only modification to production code,
inserting a sync point callback in flush_job.cc, which is not a change to
production behavior.

Removed unnecessary resetting of mock times to 0 in many tests. RocksDB
deals in relative time. Any behaviors relying on absolute date/time are likely
a bug. (The above test DBSSTTest.GetTotalSstFilesSize was the only one
clearly injecting a specific absolute time for actual testing convenience.) Just
in case I misunderstood some test, I put this note in each replacement:
// NOTE: Presumed unnecessary and removed: resetting mock time in env

Strengthened some tests like MergeTestTime, MergeCompactionTimeTest, and
FilterCompactionTimeTest in db_test.cc

stats_history_test and blob_db_test are each their own beast, rather deeply
dependent on MockTimeEnv. Each gets its own variant of a work-around for
TimedWait in a mock time environment. (Reduces redundancy and
inconsistency in stats_history_test.)

Intended follow-up:

Remove TimedWait from the public API of InstrumentedCondVar, and only
make that accessible through Env by passing in an InstrumentedCondVar and
a deadline. Then the Env implementations mocking time can fix this problem
without using sync points. (Test infrastructure using sync points interferes
with individual tests' control over sync points.)

With that change, we can simplify/consolidate the scattered work-arounds.

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

Test Plan: make check on Linux and MacOS

Reviewed By: zhichao-cao

Differential Revision: D23032815

Pulled By: pdillinger

fbshipit-source-id: 7f33967ada8b83011fb54e8279365c008bd6610b
2020-08-11 12:41:30 -07:00
Levi Tamasi
a99fb67233 Remove redundant consistency check from VersionStorageInfo::AddFile (#7237)
Summary:
`VersionStorageInfo::AddFile` currently has a debug-mode consistency
check to make sure the newly added file does not overlap with the
previous one (for levels below L0). Considering that
`VersionBuilder::CheckConsistency` also performs similar checks (in
fact, those checks are more comprehensive and cover L0 as well), this
check is redundant. The patch removes it and also cleans up `AddFile` a
little.

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

Test Plan: `make check`

Reviewed By: riversand963

Differential Revision: D23041937

Pulled By: ltamasi

fbshipit-source-id: e00665f3b83bfd17f86c54c238800f3d77d739bd
2020-08-11 09:23:17 -07:00
Andrew Kryczka
7eebe6d38a Mark files for compaction in stress/crash tests (#7231)
Summary:
The mechanism to mark files for compaction is most commonly used in
delete-triggered compaction. This PR adds an option to exercise the
marking mechanism on random files created by db_stress. This PR also
enables that option in db_crashtest.py on its db_stress runs at random.

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

Test Plan:
- ran some minified crash tests; verified they succeed and we see `"compaction_reason": "FilesMarkedForCompaction"` regularly in the logs.

```
$ TEST_TMPDIR=/dev/shm python tools/db_crashtest.py blackbox --duration=600 --interval=30 --max_key=10000000 --write_buffer_size=1048576 --target_file_size_base=1048576 --max_bytes_for_level_base=4194304 --value_size_mult=33
$ TEST_TMPDIR=/dev/shm python tools/db_crashtest.py whitebox --duration=600 --interval=30 --max_key=1000000 --write_buffer_size=1048576 --target_file_size_base=1048576 --max_bytes_for_level_base=4194304 --value_size_mult=33 --random_kill_odd=8887
```

Reviewed By: anand1976

Differential Revision: D23025156

Pulled By: ajkr

fbshipit-source-id: a404c467ebc12afa94dae35956ea9b372f592a96
2020-08-10 16:17:56 -07:00
anand76
f308da5273 Fix delete triggered compaction for single level universal (#7224)
Summary:
Delete triggered compaction (DTC) for universal compaction style with ```num_levels = 1``` has been disabled for sometime due to a data correctness bug. This PR re-enables it with a bug fix. A file marked for compaction can be picked, along with all L0 files after it as the compaction input. We stop adding files to the input once we encounter a file already being compacted (the original bug failed to check the compaction status of the files).

Tests:
Add unit tests to ```compaction_picker_test.cc```

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

Reviewed By: ajkr

Differential Revision: D23031845

Pulled By: anand1976

fbshipit-source-id: 9de3cab5f9774cede666c2c48d309a7d9b88a505
2020-08-10 12:19:17 -07:00
Andrew Kryczka
6e99de6d3c Allow optimization level to be set in Makefile (#7202)
Summary:
`-O3` is already adopted widely, so we should make it easier to configure
for development/open source. This PR adds an `OPTIMIZE_LEVEL` variable
that users can set to override the `-O` flag chosen in the Makefile.

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

Test Plan: built a few different ways and verified correct value is passed for `-O` flag

Reviewed By: pdillinger

Differential Revision: D22845291

Pulled By: ajkr

fbshipit-source-id: 84471362e7d627dd606b25bf5f6a3d796817fa1c
2020-08-10 11:25:20 -07:00
Peter Dillinger
cb26c8cc80 CircleCI: apt retries for downloads from unreliable llvm.org (#7234)
Summary:
Trying to fix issue that caused two failures out of eight most
recent master builds

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

Test Plan: Watch CircleCI

Reviewed By: riversand963

Differential Revision: D23032184

Pulled By: pdillinger

fbshipit-source-id: dae403f63c0e4f6ab8a3e8e49a49069a532b8f4a
2020-08-10 11:09:40 -07:00
Yuhong Guo
5444942f15 Fix cmake build on MacOS (#7205)
Summary:
1. `std::random_shuffle` is deprecated and now we can use `std::shuffle`
```
/rocksdb/db/prefix_test.cc:590:12: error: 'random_shuffle<std::__1::__wrap_iter<unsigned long long *> >'
      is deprecated [-Werror,-Wdeprecated-declarations]
      std::random_shuffle(prefixes.begin(), prefixes.end());
           ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/algorithm:2982:1: note:
      'random_shuffle<std::__1::__wrap_iter<unsigned long long *> >' has been explicitly marked deprecated here
_LIBCPP_DEPRECATED_IN_CXX14 void
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/__config:1107:39: note: expanded from macro
      '_LIBCPP_DEPRECATED_IN_CXX14'
#  define _LIBCPP_DEPRECATED_IN_CXX14 _LIBCPP_DEPRECATED
                                      ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/__config:1090:48: note: expanded from macro
      '_LIBCPP_DEPRECATED'
#    define _LIBCPP_DEPRECATED __attribute__ ((deprecated))
```
2. `c_test` link error with `-DROCKSDB_BUILD_SHARED=OFF`:
```
[  7%] Linking CXX executable c_test
ld: library not found for -lrocksdb-shared
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[5]: *** [c_test] Error 1
make[4]: *** [CMakeFiles/c_test.dir/all] Error 2
make[4]: *** Waiting for unfinished jobs....
```

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

Reviewed By: ajkr

Differential Revision: D23030641

Pulled By: pdillinger

fbshipit-source-id: f270e50fc0b824ca1a0876ec5c65d33f55a72dd0
2020-08-10 10:48:05 -07:00
Remington Brasga
633bff2f19 Fixed typo on Value mismatch error in db_test (#6587)
Summary:
The debug is supposed to print out two keys to show the value mismatch, which was compared just a few lines above.

However, the actual print-out is the same values (so they obviously won't be mismatched)

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

Reviewed By: riversand963

Differential Revision: D23025279

Pulled By: ajkr

fbshipit-source-id: 4c6c35bc60b273f13c08b5464b6f690d8a5cfe41
2020-08-10 10:06:08 -07:00
Yanqin Jin
8a1da56b96 Include 6.12.fb branch in format compatibility check (#7226)
Summary:
As title.

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

Test Plan:
```
tools/check_format_compatible.sh
```

Reviewed By: pdillinger

Differential Revision: D23006013

Pulled By: riversand963

fbshipit-source-id: 428f29ad984691183beae72d3d3e753ec9b085a7
2020-08-07 14:20:06 -07:00
anand76
832b056a30 Enable IO timeouts for iterators (#7161)
Summary:
Introduce io_timeout in ReadOptions and enabled deadline/io_timeout for
Iterators.

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

Test Plan: New unit tests in db_basic_test

Reviewed By: riversand963

Differential Revision: D22687352

Pulled By: anand1976

fbshipit-source-id: 67bbb0e6d7ae80b256589244468494292538c6ec
2020-08-07 12:01:08 -07:00
Zhichao Cao
b79f13b2aa Fix the potential deadlock in WriteImplWALOnly and UnorderedWriteMemtable (#7199)
Summary:
Pointed out by https://github.com/facebook/rocksdb/issues/7197 , there is a double lock in WriteImplWALOnly.
Also find another deadlock in UnorderedWriteMemtable. Move the check after switch_all_.notify_all().

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

Test Plan: pass make check

Reviewed By: anand1976

Differential Revision: D22961714

Pulled By: zhichao-cao

fbshipit-source-id: 0707922dc50d28ea141a15a8cdcbd1c8993ea0d8
2020-08-07 11:28:49 -07:00
mrambacher
56f468b356 Add more tests to ASSERT_STATUS_CHECKED (#7211)
Summary:
Added 4 more tests to those which pass ASSERT_STATUS_CHECKED (cache_test, lru_cache_test, filename_test, filelock_test).

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

Reviewed By: ajkr

Differential Revision: D22982858

Pulled By: zhichao-cao

fbshipit-source-id: acdd071582ed6aa7447ed96c5732f10bf720d783
2020-08-06 17:19:41 -07:00
Yingchun Lai
67bbac3621 Remove duplicate colon in Status message (#7041)
Summary:
A colon will be added after 'msg' automatically when invoke function Status(Code _code, const Slice& msg, const Slice& msg2),
it's not needed to append a colon explicitly to 'msg'.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/7041

Reviewed By: ajkr

Differential Revision: D22292801

fbshipit-source-id: 8f2d69065bb779d2613468bf9fc9169f32c3f1ec
2020-08-06 15:18:04 -07:00
jsteemann
5e1808d515 fix typo: paraniod -> paranoid (#7163)
Summary:
Rename "paraniod" to "paranoid" in a few places.

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

Reviewed By: ajkr

Differential Revision: D22678242

fbshipit-source-id: 28b1011a736d0a95612676f7e1b9500a70c324b4
2020-08-06 14:25:34 -07:00
Adam Retter
3356187617 Automatically number the Maven artifacts (#7219)
Summary:
Improvements to the RocksJava release process:
* Generates the Maven artifact version number as part of the release step
* Also generates appropriate checksum files to speed the deploy and publish step

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

Reviewed By: ltamasi

Differential Revision: D22983481

Pulled By: pdillinger

fbshipit-source-id: 7b8ffaf46471cd3cda181eb830c962b317d2e688
2020-08-06 14:13:30 -07:00
Cheng Chang
71c7e4935e Replace tracked_keys with a new LockTracker interface in TransactionDB (#7013)
Summary:
We're going to support more locking protocols such as range lock in transaction.

However, in current design, `TransactionBase` has a member `tracked_keys` which assumes that point lock (lock a single key) is used, and is used in snapshot checking (isolation protocol). When using range lock, we may use read committed instead of snapshot checking as the isolation protocol.

The most significant usage scenarios of `tracked_keys` are:
1. pessimistic transaction uses it to track the locked keys, and unlock these keys when commit or rollback.
2. optimistic transaction does not lock keys upfront, it only tracks the lock intentions in tracked_keys, and do write conflict checking when commit.
3. each `SavePoint` tracks the keys that are locked since the `SavePoint`, `RollbackToSavePoint` or `PopSavePoint` relies on both the tracked keys in `SavePoint`s and `tracked_keys`.

Based on these scenarios, if we can abstract out a `LockTracker` interface to hold a set of tracked locks (can be keys or key ranges), and have methods that can be composed together to implement the scenarios, then `tracked_keys` can be an internal data structure of one implementation of `LockTracker`. See `utilities/transactions/lock/lock_tracker.h` for the detailed interface design, and `utilities/transactions/lock/point_lock_tracker.cc` for the implementation.

In the future, a `RangeLockTracker` can be implemented to track range locks without affecting other components.

After this PR, a clean interface for lock manager should be possible, and then ideally, we can have pluggable locking protocols.

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

Test Plan: Run `transaction_test` and `optimistic_transaction_test`.

Reviewed By: ajkr

Differential Revision: D22163706

Pulled By: cheng-chang

fbshipit-source-id: f2860577b5334e31dd2994f5bc6d7c40d502b1b4
2020-08-06 12:38:00 -07:00