Commit Graph

10483 Commits

Author SHA1 Message Date
Hui Xiao
ffd6085e1f Add new API CacheReservationManager::GetTotalMemoryUsage() (#9071)
Summary:
Note: This PR is the 2nd PR of a bigger PR stack (https://github.com/facebook/rocksdb/pull/9073).

Context:
`CacheReservationManager::UpdateCacheReservation(std::size_t new_memory_used)` accepts an accumulated total memory used (e.g, used 10MB so far) instead of usage change (e.g, increase by 5 MB, decrease by 5 MB). It has benefits including consolidating API for increase and decrease as described in https://github.com/facebook/rocksdb/pull/8506.

However, not every `CacheReservationManager` user keeps track of this accumulated total memory usage. For example, Bloom/Ribbon Filter construction (e.g, [here](822d729fcd/table/block_based/filter_policy.cc (L587)) in https://github.com/facebook/rocksdb/pull/9073) does not  while WriteBufferManager and compression dictionary buffering do.

Considering future users might or might not keep track of this counter and implementing this counter within `CacheReservationManager` is easy due to the passed-in `std::size_t new_memory_used` in calling `CacheReservationManager::UpdateCacheReservation(std::size_t new_memory_used)`, it is proposed to add a new API `CacheReservationManager::GetTotalMemoryUsage()`.

As noted in the API comments,   since `CacheReservationManager` is NOT thread-safe, external synchronization is
 needed in calling `UpdateCacheReservation()` if you want `GetTotalMemoryUsed()` returns the indeed latest memory used.
- Added and updated private counter `memory_used_` every time `CacheReservationManager::UpdateCacheReservation(std::size_t new_memory_used)` is called regardless if the call returns non-okay status
- Added `CacheReservationManager::GetTotalMemoryUsage()` to return `memory_used_`

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

Test Plan:
- Passing new tests
- Passing existing tests

Reviewed By: ajkr

Differential Revision: D31887813

Pulled By: hx235

fbshipit-source-id: 9a09f0c8683822673260362894c878b61ee60ceb
2021-11-09 08:17:03 -08:00
Zhichao Cao
efaef9b40a cleanup error_handler related code (#9098)
Summary:
Remove code not in use, add comments, remove redundant code.

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

Test Plan: make check

Reviewed By: anand1976

Differential Revision: D32027219

Pulled By: zhichao-cao

fbshipit-source-id: 253aae926c87726268af6c027bf805dc9156c8a8
2021-11-08 15:49:17 -08:00
jsteemann
a7478070f3 Fix small issues (#5896)
Summary:
The individual commits in this PR should be self-explanatory.
All small and _very_ low-priority changes.

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

Reviewed By: riversand963

Differential Revision: D18065108

Pulled By: mrambacher

fbshipit-source-id: 236b1a1d9d21f982cc08aa67027108dde5eaf280
2021-11-08 12:32:38 -08:00
anand76
dddb791c18 Enable a few unit tests to use custom Env objects (#9087)
Summary:
Allow compaction_job_test, db_io_failure_test, dbformat_test, deletefile_test, and fault_injection_test to use a custom Env object. Also move ```RegisterCustomObjects``` declaration to a header file to simplify things.

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

Test Plan: Run manually using "buck test rocksdb/src:compaction_job_test_fbcode" etc.

Reviewed By: riversand963

Differential Revision: D32007222

Pulled By: anand1976

fbshipit-source-id: 99af58559e25bf61563dfa95dc46e31fa7375792
2021-11-08 11:05:59 -08:00
anand76
78556c14dd Secondary cache error injection (#9002)
Summary:
Implement secondary cache error injection in db_stress.

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

Reviewed By: zhichao-cao

Differential Revision: D31874896

Pulled By: anand1976

fbshipit-source-id: 8cf04c061a4a44efa0fe88423d05cade67b85f73
2021-11-08 10:27:27 -08:00
Alan Paxton
e5b34f5867 Fb 5789 max total WAL size clarification (#9108)
Summary:
Add clarification/extension to comments on max_total_wal_size and the Java wrapper MaxTotalWalSize to better explain the effect of the option on log file sizes.

Closes https://github.com/facebook/rocksdb/issues/5789

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

Reviewed By: pdillinger

Differential Revision: D32066640

Pulled By: mrambacher

fbshipit-source-id: 7d5affc87e4119019054af9c884a2ea01d68f5b7
2021-11-08 08:54:37 -08:00
Adam Retter
be351f4754 Restore Java 7 Compatibility (#9103)
Summary:
RocksDB should still compile on Java 7.

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

Reviewed By: pdillinger

Differential Revision: D32067561

Pulled By: mrambacher

fbshipit-source-id: bbe9c18c8007ab3e113de4add56a84c9bde61c8e
2021-11-08 08:21:02 -08:00
Yanqin Jin
8ef5b9ddfe Update MySQLStyleTransactionTest to use SingleDelete (#9062)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9062

Real MySQL-style transactions in MyRocks uses SingleDelete, which is
missing in our existint MySQLStyleTransactionTest. Ths diff by lth fills the
gap in test coverage.

Reviewed By: lth

Differential Revision: D31813015

fbshipit-source-id: 196ad761de30ae9ea1f92257058dfc265f211892
2021-11-07 06:41:12 -08:00
Dennis Maisenbacher
9e788be4b7 Source files dependencies detection for RocksDB plugins. (#9120)
Summary:
Otherwise a rebuild is not done if a RocksDB plugin header file is
changed.

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

Test Plan:
Build RocksDB with a plugin.
Change a header file of the RocksDB plugin and rebuild.

Signed-off-by: Dennis Maisenbacher <dennis.maisenbacher@wdc.com>

Reviewed By: riversand963

Differential Revision: D32223303

Pulled By: ajkr

fbshipit-source-id: 76d31b10fe915906edc181c7b6398a09b7d079ee
2021-11-06 11:50:17 -07:00
Jay Zhuang
5aad38f262 Deflake DBBasicTestWithTimestampCompressionSettings.PutAndGetWithComp… (#9136)
Summary:
…action
```
db/db_with_timestamp_basic_test.cc:2643: Failure
db_->CompactFiles(compact_opt, handles_[cf], collector->GetFlushedFiles(), static_cast<int>(kNumTimestamps - i))
Invalid argument: A compaction must contain at least one file.
```
Able to be reproduced by run multiple test in parallel.

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

Test Plan:
```
gtest-parallel ./db_with_timestamp_basic_test --gtest_filter=Timestamp/DBBasicTestWithTimestampCompressionSettings.PutAndGetWithCompaction/12 -r 100 -w 100
```

Reviewed By: riversand963

Differential Revision: D32197734

Pulled By: jay-zhuang

fbshipit-source-id: aeb0d6e9b37312f577e203ca81bb7a0f14d4e7ce
2021-11-05 17:57:50 -07:00
Levi Tamasi
3fbddb1d27 Refactor and unify blob file saving and the logic that finds the oldest live blob file (#9122)
Summary:
The patch refactors and unifies the logic in `VersionBuilder::SaveBlobFilesTo`
and `VersionBuilder::GetMinOldestBlobFileNumber` by introducing a generic
helper that can "merge" the list of `BlobFileMetaData` in the base version with
the list of `MutableBlobFileMetaData` representing the updated state after
applying a sequence of `VersionEdit`s. This serves as groundwork for subsequent
changes that will enable us to determine whether a blob file is live after applying
a sequence of edits without calling `VersionBuilder::SaveTo`.

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

Test Plan: `make check`

Reviewed By: riversand963

Differential Revision: D32151472

Pulled By: ltamasi

fbshipit-source-id: 11622b475866de823334b8bc21b0e99d913af97e
2021-11-05 16:50:52 -07:00
Hui Xiao
3018a3e27e Minor improvement to CacheReservationManager/WriteBufferManager/CompressionDictBuilding (#9139)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/9139

Reviewed By: zhichao-cao

Differential Revision: D32211415

Pulled By: hx235

fbshipit-source-id: 39ce036ba34e1fb4a1992a33ac6904a4a943301d
2021-11-05 16:13:47 -07:00
Yanqin Jin
5237b39d2e Fix assertion error during compaction with write-prepared txn enabled (#9105)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9105

The user contract of SingleDelete is that: a SingleDelete can only be issued to
a key that exists and has NOT been updated. For example, application can insert
one key `key`, and uses a SingleDelete to delete it in the future. The `key`
cannot be updated or removed using Delete.
In reality, especially when write-prepared transaction is being used, things
can get tricky. For example, a prepared transaction already writes `key` to the
memtable after a successful Prepare(). Afterwards, should the transaction
rollback, it will insert a Delete into the memtable to cancel out the prior
Put. Consider the following sequence of operations.

```
// operation sequence 1
Begin txn
Put(key)
Prepare()
Flush()

Rollback txn
Flush()
```

There will be two SSTs resulting from above. One of the contains a PUT, while
the second one contains a Delete. It is also known that releasing a snapshot
can lead to an L0 containing only a SD for a particular key. Consider the
following operations following the above block.

```
// operation sequence 2
db->Put(key)
db->SingleDelete(key)
Flush()
```

The operation sequence 2 can result in an L0 with only the SD.

Should there be a snapshot for conflict checking created before operation
sequence 1, then an attempt to compact the db may hit the assertion failure
below, because ikey_.type is Delete (from a rollback).

```
else if (clear_and_output_next_key_) {
  assert(ikey_.type == kTypeValue || ikey_.type == kTypeBlobIndex);
}
```

To fix the assertion failure, we can skip the SingleDelete if we detect an
earlier Delete in the same snapshot interval.

Reviewed By: ltamasi

Differential Revision: D32056848

fbshipit-source-id: 23620a91e28562d91c45cf7e95f414b54b729748
2021-11-05 15:29:18 -07:00
Jack Feng
7d74d3471c Fixing more loosely formatted issue (#9140)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9140

Upon checking, some jobs still have loosely formatted issue.
Also, some started failing due to my previous buggy change.

https://fburl.com/scuba/sandcastle/e9mhejsc
https://fburl.com/scuba/duplo_events/dbxx4215

Reviewed By: jay-zhuang

Differential Revision: D32213703

fbshipit-source-id: 806f872b820afe275cf62459988c9d6f668af35c
2021-11-05 12:21:38 -07:00
Siying Dong
28bab0ef7e Improve comments on options.writable_file_max_buffer_size (#9131)
Summary:
Comments of options.writable_file_max_buffer_size mentioned Windows, which is confusing. Remove it.

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

Reviewed By: anand1976

Differential Revision: D32187003

fbshipit-source-id: 1f134d7ecdc4a9d13825d461ab1da56769b9455f
2021-11-04 16:38:09 -07:00
sdong
caadf09d53 Add options.manual_wal_flush to db_bench (#9132)
Summary:
It is useful to add options.manual_wal_flush to db_bench

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

Test Plan: Run the benchamrk with the option.

Reviewed By: ltamasi

Differential Revision: D32188060

fbshipit-source-id: a70835d3cad0f30095218dfda1daff0a432892e5
2021-11-04 16:03:47 -07:00
Jack Feng
8be121bb80 Fixing loosely formatted child job specs, double quotes (#9125)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9125

See task for more context

Reviewed By: jay-zhuang

Differential Revision: D32157345

fbshipit-source-id: be6631c6400fd66e6891e71dc0235798c594010a
2021-11-04 15:34:51 -07:00
Peter Dillinger
2a3511a0df Fix -Werror=type-limits seen in Travis (#9128)
Summary:
Work around annoying compiler warning-as-error from https://github.com/facebook/rocksdb/issues/9113

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

Test Plan: CI

Reviewed By: jay-zhuang

Differential Revision: D32181499

Pulled By: pdillinger

fbshipit-source-id: d7e5f7857a29f7ba47c49c3aee7150b5763b65d9
2021-11-04 15:01:10 -07:00
Hui Xiao
1ababeb76a Deallocate payload of BlockBasedTableBuilder::Rep::FilterBlockBuilder earlier for Full/PartitionedFilter (#9070)
Summary:
Note: This PR is the 1st part of a bigger PR stack (https://github.com/facebook/rocksdb/pull/9073).

Context:
Previously, the payload (i.e, filter data) within `BlockBasedTableBuilder::Rep::FilterBlockBuilder` object is not deallocated until `BlockBasedTableBuilder` is deallocated, despite it is no longer useful after its related `filter_content` being written.
- Transferred the payload (i.e, the filter data) out of `BlockBasedTableBuilder::Rep::FilterBlockBuilder` object
- For PartitionedFilter:
   - Unified `filters` and `filter_gc` lists into one `std::deque<FilterEntry> filters` by adding a new field `last_filter_entry_key` and storing the `std::unique_ptr filter_data` with the `Slice filter` in the same entry
   - Reset `last_filter_data` in the case where `filters` is empty, which should be as by then we would've finish using all the `Slice filter`
- Deallocated the payload by going out of scope as soon as we're done with using the `filter_content` associated with the payload
- This is an internal interface change at the level of `FilterBlockBuilder::Finish()`, which leads to touching the inherited interface in `BlockBasedFilterBlockBuilder`. But for that, the payload transferring is ignored.

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

Test Plan: - The main focus is to catch segment fault error during `FilterBlockBuilder::Finish()` and `BlockBasedTableBuilder::Finish()` and interface mismatch. Relying on existing CI tests is enough as `assert(false)` was temporarily added to verify the new logic of transferring ownership indeed run

Reviewed By: pdillinger

Differential Revision: D31884933

Pulled By: hx235

fbshipit-source-id: f73ecfbea13788d4fc058013ace27230110b52f4
2021-11-04 13:35:38 -07:00
Hui Xiao
a64c8ca7a8 Sanitize negative request bytes in GenericRateLimiter::Request and clarify API (#9112)
Summary:
Context:
Surprisingly, there isn't any sanitization against negative `int64_t bytes` in `GenericRateLimiter::Request(int64_t bytes, const Env::IOPriority pri, Statistics* stats)`. A negative `bytes` can be passed in and incorrectly increases `available_bytes_` by subtracting the negative `bytes` from `available_bytes_`, such as  [here](https://github.com/facebook/rocksdb/blob/main/util/rate_limiter.cc#L138) and [here](https://github.com/facebook/rocksdb/blob/main/util/rate_limiter.cc#L283), which are incorrect behaviors.
- Sanitized negative request bytes by rounding it up to 0
- Added notes to public and internal API

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

Test Plan: - Rely on existing tests

Reviewed By: ajkr

Differential Revision: D32085364

Pulled By: hx235

fbshipit-source-id: b1b6066b2dd5ffc7bcbfb07069ca65a33578251b
2021-11-04 10:11:53 -07:00
Peter Dillinger
dfedc74d82 Some checksum code refactoring (#9113)
Summary:
To prepare for adding checksum to footer and "context aware"
checksums. This also brings closely related code much closer together.

Recently added `BlockBasedTableBuilder::ComputeBlockTrailer` for testing
is made obsolete in the refactoring, as testing the checksums can happen
at a lower level of abstraction.

Also now checking for unrecognized checksum type on reading footer,
rather than later on use.

Also removed an obsolete function delcaration.

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

Test Plan:
existing tests worked before refactoring to remove
`ComputeBlockTrailer`. And then refactored+improved tests using it.

Reviewed By: mrambacher

Differential Revision: D32090149

Pulled By: pdillinger

fbshipit-source-id: 2879da683c1498ea85a3b70dace9b6d9f6b47b6e
2021-11-04 09:09:34 -07:00
Adam Retter
312d9c47b4 Re-enable 390x+cmake* Travis jobs (#9110)
Summary:
Revert "Temporarily disable s390x+cmake* Travis jobs (https://github.com/facebook/rocksdb/issues/9095)"
This reverts commit f2d11b3fdc.

I have now uploaded the CMake deb for s390x provided by jonathan-albrecht-ibm to our S3 bucket.

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

Reviewed By: ajkr

Differential Revision: D32082903

Pulled By: riversand963

fbshipit-source-id: b7243d19fc133e665a8654e3b528c4f53d5b11d1
2021-11-03 20:30:15 -07:00
Yanqin Jin
9b53f14a35 Fixed a bug in CompactionIterator when write-preared transaction is used (#9060)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9060

RocksDB bottommost level compaction may zero out an internal key's sequence if
the key's sequence is in the earliest_snapshot.
In write-prepared transaction, checking the visibility of a certain sequence in
a specific released snapshot may return a "snapshot released" result.
Therefore, it is possible, after a certain sequence of events, a PUT has its
sequence zeroed out, but a subsequent SingleDelete of the same key will still
be output with its original sequence. This violates the ascending order of
keys and leads to incorrect result.

The solution is to use an extra variable `last_key_seq_zeroed_` to track the
information about visibility in earliest snapshot. With this variable, we can
know for sure that a SingleDelete is in the earliest snapshot even if the said
snapshot is released during compaction before processing the SD.

Reviewed By: ltamasi

Differential Revision: D31813016

fbshipit-source-id: d8cff59d6f34e0bdf282614034aaea99be9174e1
2021-11-03 15:55:00 -07:00
Jack Feng
56810142a5 Fixing child jobs of rocksdb for duplo deprecation (#9117)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9117

This fixes the loosely formatted json child job spec. I.e., trailing commas

Reviewed By: jay-zhuang

Differential Revision: D32119606

fbshipit-source-id: 0ef571ccbef0e638df18f17288611d3b5774f129
2021-11-03 14:56:26 -07:00
Jay Zhuang
29102641dd Skip directory fsync for filesystem btrfs (#8903)
Summary:
Directory fsync might be expensive on btrfs and it may not be needed.
Here are 4 directory fsync cases:
1. creating a new file: dir-fsync is not needed on btrfs, as long as the
   new file itself is synced.
2. renaming a file: dir-fsync is not needed if the renamed file is
   synced. So an API `FsyncAfterFileRename(filename, ...)` is provided
   to sync the file on btrfs. By default, it just calls dir-fsync.
3. deleting files: dir-fsync is forced by set
   `IOOptions.force_dir_fsync = true`
4. renaming multiple files (like backup and checkpoint): dir-fsync is
   forced, the same as above.

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

Test Plan: run tests on btrfs and non btrfs

Reviewed By: ajkr

Differential Revision: D30885059

Pulled By: jay-zhuang

fbshipit-source-id: dd2730b31580b0bcaedffc318a762d7dbf25de4a
2021-11-03 12:21:27 -07:00
Levi Tamasi
081722780b Refactor the detailed consistency checks and the SST saving logic in VersionBuilder (#9099)
Summary:
The patch refactors the parts of `VersionBuilder` that deal with SST file
comparisons. Specifically, it makes the following changes:
* Turns `NewestFirstBySeqNo` and `BySmallestKey` from free-standing
functions into function objects. Note: `BySmallestKey` has a pointer to the
`InternalKeyComparator`, while `NewestFirstBySeqNo` is completely
stateless.
* Eliminates the wrapper `FileComparator`, which was essentially an
unnecessary DIY virtual function call mechanism.
* Refactors `CheckConsistencyDetails` and `SaveSSTFilesTo` using helper
function templates that take comparator/checker function objects. Using
static polymorphism eliminates the need to make runtime decisions about
which comparator to use.
* Extends some error messages returned by the consistency checks and
makes them more uniform.
* Removes some incomplete/redundant consistency checks from `VersionBuilder`
and `FilePicker`.
* Improves const correctness in several places.

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

Test Plan: `make check`

Reviewed By: riversand963

Differential Revision: D32027503

Pulled By: ltamasi

fbshipit-source-id: 621326ae41f4f55f7ad6a91abbd6e666d5c7857c
2021-11-03 11:52:47 -07:00
Peter Dillinger
2b60621f16 Don't call OnTableFileCreated with OK for empty+deleted file (#9118)
Summary:
EventListener::OnTableFileCreated was previously called with OK
status and file_size==0 in cases of no SST file contents written
(because there was no content to add) and the empty file deleted before
calling the listener. This could lead to a stress test assertion failure
added in https://github.com/facebook/rocksdb/issues/9054.

This changes the status to Aborted, to align with the API doc:
"... if the file is successfully created. Now it will also be called on
failure case. User can check info.status to see if it succeeded or not."
For internal purposes, this case is considered "success" but for
listener purposes, no SST file is (successfully) created.

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

Test Plan: test case added + existing db_stress

Reviewed By: ajkr, riversand963

Differential Revision: D32120232

Pulled By: pdillinger

fbshipit-source-id: a804e2e0a52598018d3b182da97804d402ffcdfa
2021-11-03 08:43:27 -07:00
Peter Dillinger
21f8a57f2a Fix TSAN report on MemPurge test (#9115)
Summary:
TSAN reported data race on count variables in MemPurgeBasic
test. This suggests the test could fail if mempurges were slow enough
that they don't complete before the count variables being checked, but
injecting a long sleep into MemPurge (outside DB mutex) confirms that
blocked writes ensure enough mempurges/flushes happen to make the test
pass. All the possible different values on testing should be OK to make
the test pass.

So this change makes the variables atomic so that up-to-date value is
always read and TSAN report suppressed. I have also used `.exchange(0)`
to make the checking less stateful by "popping off" all the accumulated
counts.

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

Test Plan: updated test, watch for any flakiness

Reviewed By: riversand963

Differential Revision: D32114432

Pulled By: pdillinger

fbshipit-source-id: c985609d39896a0d8f69ebc87b221e688609bdd8
2021-11-02 21:54:29 -07:00
Andrew Kryczka
67a7b74b7f Clarify setting CompressionOptions::max_dict_bytes > 0 will charge block cache (#9119)
Summary:
Add it to the API comment.

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

Reviewed By: hx235

Differential Revision: D32124238

Pulled By: ajkr

fbshipit-source-id: d1f82037417d883f2000f2d62995a7708dda77c6
2021-11-02 21:43:50 -07:00
Peter Dillinger
82afa01815 Some API clarifications (#9080)
Summary:
* Clarify that RocksDB is not exception safe on many of our callback
and extension interfaces
* Clarify FSRandomAccessFile::MultiRead implementations must accept
non-sorted inputs (see https://github.com/facebook/rocksdb/issues/8953)
* Clarify ConcurrentTaskLimiter and SstFileManager are not (currently)
extensible interfaces
* Mark WriteBufferManager as `final`, so it is then clearly not a
callback interface, even though it smells like one
* Clarify TablePropertiesCollector Status returns are mostly ignored

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

Test Plan: comments only (except WriteBufferManager final)

Reviewed By: ajkr

Differential Revision: D31968782

Pulled By: pdillinger

fbshipit-source-id: 11b648ce3ce3c5e5bdc02d2eafc7ea4b864bd1d2
2021-11-02 20:30:07 -07:00
mrambacher
f72c834eab Make FileSystem a Customizable Class (#8649)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/8649

Reviewed By: zhichao-cao

Differential Revision: D32036059

Pulled By: mrambacher

fbshipit-source-id: 4f1e7557ecac52eb849b83ae02b8d7d232112295
2021-11-02 09:07:11 -07:00
Levi Tamasi
cfc57f55b5 Mention PR 9100 in HISTORY.md (#9111)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/9111

Reviewed By: riversand963

Differential Revision: D32076310

Pulled By: ltamasi

fbshipit-source-id: 81e30a02ded87c0f1a42985db42e80b62235ba11
2021-11-01 15:22:32 -07:00
Alan Paxton
ec9082d698 Regression tests for tickets fixed by previous change. (#9019)
Summary:
closes https://github.com/facebook/rocksdb/issues/5891
closes https://github.com/facebook/rocksdb/issues/2001

Java BytewiseComparator is now unsigned compliant, consistent with the default C++ comparator, which has always been thus. Consequently 2 tickets reporting the previous broken state can be closed.

 This test confirms that the following issues were in fact resolved
 by a change made between 6.2.2 and 6.22.1,
 to wit https://github.com/facebook/rocksdb/commit/7242dae7
which as part of its effect, changed the Java bytewise comparators.

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

Reviewed By: pdillinger

Differential Revision: D31610910

Pulled By: mrambacher

fbshipit-source-id: 664230f1377a1aa270136edd63eea2c206b907e9
2021-11-01 15:06:47 -07:00
Hui Xiao
560fe70233 Add new API CacheReservationManager::GetDummyEntrySize() (#9072)
Summary:
Note: it might conflict with another CRM related PR https://github.com/facebook/rocksdb/pull/9071 and so will merge after that's merged.

Context:
As `CacheReservationManager` being used by more memory users, it is convenient to retrieve the dummy entry size for `CacheReservationManager` instead of hard-coding `256 * 1024` in writing tests. Plus it allows more flexibility to change our implementation on dummy entry size.

A follow-up PR is needed to replace those hard-coded dummy entry size value in `db_test2.cc`, `db_write_buffer_manager_test.cc`, `write_buffer_manager_test.cc`, `table_test.cc` and the ones introduced in https://github.com/facebook/rocksdb/pull/9072#issue-1034326069.
- Exposed the private static constexpr `kDummyEntrySize` through public static `CacheReservationManager::GetDummyEntrySize()`

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

Test Plan:
- Passing new tests
- Passing existing tests

Reviewed By: ajkr

Differential Revision: D32043684

Pulled By: hx235

fbshipit-source-id: ddefc6921c052adab6a2cda2394eb26da3076a50
2021-11-01 14:46:09 -07:00
sdong
a2b9be42b6 Try to start TTL earlier with kMinOverlappingRatio is used (#8749)
Summary:
Right now, when options.ttl is set, compactions are triggered around the time when TTL is reached. This might cause extra compactions which are often bursty. This commit tries to mitigate it by picking those files earlier in normal compaction picking process. This is only implemented using kMinOverlappingRatio with Leveled compaction as it is the default value and it is more complicated to change other styles.

When a file is aged more than ttl/2, RocksDB starts to boost the compaction priority of files in normal compaction picking process, and hope by the time TTL is reached, very few extra compaction is needed.

In order for this to work, another change is made: during a compaction, if an output level file is older than ttl/2, cut output files based on original boundary (if it is not in the last level). This is to make sure that after an old file is moved to the next level, and new data is merged from the upper level, the new data falling into this range isn't reset with old timestamp. Without this change, in many cases, most files from one level will keep having old timestamp, even if they have newer data and we stuck in it.

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

Test Plan: Add a unit test to test the boosting logic. Will add a unit test to test it end-to-end.

Reviewed By: jay-zhuang

Differential Revision: D30735261

fbshipit-source-id: 503c2d89250b22911eb99e72b379be154de3428e
2021-11-01 14:36:31 -07:00
hx235
a5ec5e3ea0 Minor improvement to #8428 (Account for dictionary-building buffer in global memory limit) (#9032)
Summary:
Summary/Context:
- Renamed `cache_rev_mng` to `compression_dict_buffer_cache_res_mgr`
   - It is to distinguish with other potential `cache_res_mgr` in `BlockBasedTableBuilder` and to use correct short-hand for the words "reservation", "manager"
- Added `table_options.block_cache == nullptr` in additional to `table_options.no_block_cache == true` to be conditions where we don't create a `CacheReservationManager`
   - Theoretically `table_options.no_block_cache == true` is equivalent to `table_options.block_cache == nullptr` by API. But since segment fault will be generated by passing `nullptr` into `CacheReservationManager`'s constructor, it does not hurt to directly verify  `table_options.block_cache != nullptr` before passing in
- Renamed `is_cache_full` to `exceeds_global_block_cache_limit`
   - It is to hide implementation detail of cache reservation and to emphasize on the concept/design intent of caping memory within global block cache limit

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

Test Plan: - Passing existing tests

Reviewed By: ajkr

Differential Revision: D32005807

Pulled By: hx235

fbshipit-source-id: 619fd17bb924199de3db5924d8ab7dae53b1efa2
2021-11-01 14:28:09 -07:00
leipeng
230c98f3ce fix histogram NUM_FILES_IN_SINGLE_COMPACTION (#9026)
Summary:
currently histogram `NUM_FILES_IN_SINGLE_COMPACTION` just counted files in first level of compaction input, this fix counts files in all levels of compaction input.

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

Reviewed By: ajkr

Differential Revision: D31668241

Pulled By: jay-zhuang

fbshipit-source-id: c02f6c4a5df9fbf0b7510036594811152e8738af
2021-11-01 12:57:27 -07:00
Levi Tamasi
b1c27a52d2 Add a consistency check that prevents the overflow of garbage in blob files (#9100)
Summary:
The number or total size of garbage blobs in any given blob file can
never exceed the number or total size of all blobs in the file. (This
would be a similar error to e.g. attempting to remove from the LSM tree
an SST file that has already been removed.) The patch builds on
https://github.com/facebook/rocksdb/issues/9085 and adds a
consistency check to `VersionBuilder` that prevents the above from
happening.

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

Test Plan: `make check`

Reviewed By: riversand963

Differential Revision: D32048982

Pulled By: ltamasi

fbshipit-source-id: 6f7e0793bf534ad04c3359cc0f696b8e4e5ef81c
2021-11-01 12:32:14 -07:00
Alan Paxton
73e6b89fad Java wrapper for blob_gc_force_threshold as blobGarbageCollectionForceThreshold (#9109)
Summary:
Extra option added as a supplement to https://github.com/facebook/rocksdb/pull/8999

Closes https://github.com/facebook/rocksdb/issues/8221

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

Reviewed By: mrambacher

Differential Revision: D32065039

Pulled By: ltamasi

fbshipit-source-id: 6c484050a30fe0523850a8a3c95dc85b0a501362
2021-11-01 11:59:10 -07:00
leipeng
2b70224f82 remove bad extra RecordTick(stats_, WRITE_WITH_WAL) (#9064)
Summary:
This PR fix wrong ticker `WRITE_WITH_WAL`.

`RecordTick(WRITE_WITH_WAL)` will be called later in `WriteToWAL` and `ConcurrentWriteToWAL`.

Fixes:
1. Delete these two extra `RecordTick(WRITE_WITH_WAL)`
2. Fix corresponding test case

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

Reviewed By: ajkr

Differential Revision: D31944459

Pulled By: riversand963

fbshipit-source-id: f1aa8d2a4320456bc357bc5b0902032f7dcad086
2021-11-01 11:43:14 -07:00
Yanqin Jin
0e12b1d691 Update buckify scripts (#9104)
Summary:
49af999954
updates RocksDB buckifier script directly via fbcode. We need to make
sure that the following command run in RocksDB repo generate the same
TARGETS file.

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

Test Plan:
```
$python buckifier/buckify_rocksdb.py
```
Verify that TARGETS file does not have uncommitted changes.

Reviewed By: jay-zhuang

Differential Revision: D32055387

Pulled By: riversand963

fbshipit-source-id: 19cf1b8145095b6df625958458189680e543e3ba
2021-11-01 10:11:18 -07:00
leipeng
01bd86ad35 InternalStats::DumpCFMapStat: fix sum.w_amp (#9065)
Summary:
sum `w_amp` will be a very large number`(bytes_written + bytes_written_blob)` when there is no any flush and ingest.

This PR set sum `w_amp` to zero if there is no any flush and ingest, this is conform to per-level `w_amp` computation.

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

Reviewed By: ajkr

Differential Revision: D31943994

Pulled By: riversand963

fbshipit-source-id: acbef5e331debebfad09e0e0d8d0885ebbc00609
2021-10-31 23:11:43 -07:00
Yanqin Jin
d263505417 Avoid div-by-zero error in db_stress (#9086)
Summary:
If a column family has 0 levels, then existing `TestCompactFiles(...)` may hit
divide-by-zero. To fix, return early if the cf is empty.

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

Test Plan: TBD

Reviewed By: ajkr

Differential Revision: D31986799

Pulled By: riversand963

fbshipit-source-id: 48f7dfb2b2b47cfc1315cb71ca80eb230d947f17
2021-10-31 22:16:03 -07:00
Yanqin Jin
8e59a1dc9a Attempt to deflake ListenerTest.MultiCF (#9084)
Summary:
EventListenerTest.MultiCF uses TestFlushListener which has members
flushed_dbs_ and flushed_column_family_names_ that are not protected by
locks. This implicitly indicates that we need to ensure the methods
accessing these data structures in a single threaded way. In other
tests, e.g. MultiDBMultiListeners, we use TEST_WaitForFlushMemtable() to
wait until all memtables of a given column family are flushed, hence no
pending flush threads will concurrently call OnFlushCompleted() and
cause data race for flushed_dbs_. To fix a test failure, we should do
the same for MultiCF.

Example data race stack traces reported by TSAN
```
Read of size 8 at 0x7b6000002840 by main thread:
    #0 std::vector<rocksdb::DB*, std::allocator<rocksdb::DB*> >::size() const /usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_vector.h:655:40
    https://github.com/facebook/rocksdb/issues/1 rocksdb::EventListenerTest_MultiCF_Test::TestBody() /home/circleci/project/db/listener_test.cc:380:7

Previous write of size 8 at 0x7b6000002840 by thread T2:
    #0 void std::vector<rocksdb::DB*, std::allocator<rocksdb::DB*> >::_M_emplace_back_aux<rocksdb::DB* const&>(rocksdb::DB* const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/vector.tcc:442:26
    https://github.com/facebook/rocksdb/issues/1 std::vector<rocksdb::DB*, std::allocator<rocksdb::DB*> >::push_back(rocksdb::DB* const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_vector.h:923:4
    https://github.com/facebook/rocksdb/issues/2 rocksdb::TestFlushListener::OnFlushCompleted(rocksdb::DB*, rocksdb::FlushJobInfo const&) /home/circleci/project/db/listener_test.cc:255:18
```

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

Test Plan: ./listener_test --gtest_filter=EventListenerTest.MultiCF

Reviewed By: jay-zhuang

Differential Revision: D31952259

Pulled By: riversand963

fbshipit-source-id: 94a7f29e4e9466ead42418944eb2247fc32bd499
2021-10-31 22:12:15 -07:00
Yanqin Jin
8f4f302316 Attempt to deflake DBFlushTest.FireOnFlushCompletedAfterCommittedResult (#9083)
Summary:
DBFlushTest.FireOnFlushCompletedAfterCommittedResult uses test sync
points to coordinate interleaving of different threads. Before this PR,
the test writes some data to memtable, triggers a manual flush, and
triggers a second manual flush after a first bg flush thread starts
executing. Though unlikely, it is possible for the second bg flush
thread to run faster than the first bg flush thread and deques flush
queue first. In this case, the original test will fail.
The fix is to wait until the first bg flush thread deques the flush
queue before triggering second manual flush.

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

Test Plan: ./db_flush_test --gtest_filter=DBFlushTest.FireOnFlushCompletedAfterCommittedResult

Reviewed By: jay-zhuang

Differential Revision: D31951239

Pulled By: riversand963

fbshipit-source-id: f32d7cdabe6ad6808fd18e54e663936dc0a9edb4
2021-10-31 22:08:48 -07:00
CodemodService Bot
49af999954 internal_repo_rocksdb/repo
Reviewed By: DrMarcII

Differential Revision: D32033741

fbshipit-source-id: af12d9d72f109a4a2837cb64e02fa0dbc9175711
2021-10-29 19:34:39 -07:00
Levi Tamasi
44d04582cb Aggregate blob file related changes in VersionBuilder as VersionEdits are applied (#9085)
Summary:
The current VersionBuilder code on mainline keeps track of blob file related
changes ("delta") induced by a series of `VersionEdit`s in the form of
`BlobFileMetaDataDelta` objects. Specifically, `BlobFileMetaDataDelta`
contains the amount of additional garbage generated by compactions, as well
as the set of newly linked/unlinked SSTs. This is very handy for detecting trivial moves,
since in that case the newly linked and unlinked SSTs cancel each other out.
However, this representation does not allow us to easily tell whether a certain
blob file is obsolete after applying a set of `VersionEdit`s or not. In order to
solve this issue, the patch introduces `MutableBlobFileMetaData`, which, in addition
to the delta, also contains the materialized state after applying a set of version edits
(i.e. the total amount of garbage and the resulting set of linked SSTs). This will
enable us to add further consistency checks and to improve certain pieces of
functionality where knowing up front which blob files get obsoleted is beneficial.
(Note: this patch is just the refactoring part; I plan to create separate PRs for
the enhancements.)

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

Test Plan: Ran `make check` and the stress tests in BlobDB mode.

Reviewed By: riversand963

Differential Revision: D31980867

Pulled By: ltamasi

fbshipit-source-id: cc4286778b10900af720423d6b772c77f28a93e3
2021-10-29 17:47:02 -07:00
Yanqin Jin
fdf2a0d7eb Fix a compaction bug for write-prepared txn (#9061)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9061

In write-prepared txn, checking a sequence's visibility in a released (old)
snapshot may return "Snapshot released". Suppose we have two snapshots:

```
earliest_snap < earliest_write_conflict_snap
```

If we release `earliest_write_conflict_snap` but keep `earliest_snap` during
bottommost level compaction, then it is possible that certain sequence of
events can lead to a PUT being seq-zeroed followed by a SingleDelete of the
same key. This violates the ascending order of keys, and will cause data
inconsistency.

Reviewed By: ltamasi

Differential Revision: D31813017

fbshipit-source-id: dc68ba2541d1228489b93cf3edda5f37ed06f285
2021-10-29 15:23:17 -07:00
Jonathan Albrecht
f2d11b3fdc Temporarily disable s390x+cmake* Travis jobs (#9095)
Summary:
Temporarily disable s390x+cmake* jobs until a cmake-3.14.5-Linux-s390x.deb can be installed to https://rocksdb-deps.s3-us-west-2.amazonaws.com.

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

Reviewed By: akankshamahajan15

Differential Revision: D32025417

Pulled By: riversand963

fbshipit-source-id: eefb9737937987c7d9273482a89e4d2266cd5375
2021-10-29 11:06:50 -07:00
Peter Dillinger
92e2399669 Fix EnvLibrados and add to CI (#9088)
Summary:
This feature was not part of any common or CI build, so no
surprise it broke. Now we can at least ensure compilation. I don't know
how to run the test successfully (missing config file) so it is bypassed
for now.

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

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

Test Plan: CI

Reviewed By: mrambacher

Differential Revision: D32009467

Pulled By: pdillinger

fbshipit-source-id: 3e0d1e5fde7f0ece703d48a81479e1cc7392c25c
2021-10-29 08:19:03 -07:00