Summary:
As explained in the comments, Sometimes we create Slice(nullptr, 0) in our code base which cause us to do calls like
```
memcmp(nullptr, "abc", 0);
```
That's fine since the len is equal 0, but UBSAN is not happy about it
so disable UBSAN for this function and add an assert instead
Closes https://github.com/facebook/rocksdb/pull/2616
Differential Revision: D5458326
Pulled By: IslamAbdelRahman
fbshipit-source-id: cfca32abe30f7d8f760c9f77ecd9543dfb1170dd
Summary:
simply enable the macro in internal build, it wont hurt other sanitizers and will fix UBSAN issues
Closes https://github.com/facebook/rocksdb/pull/2625
Differential Revision: D5475897
Pulled By: IslamAbdelRahman
fbshipit-source-id: 262c6fd5de3c1906f4b29e55b39110f125f41057
Summary:
Add and implement Iterator::Refresh(). When this function is called, if the super version doesn't change, update the sequence number of the iterator to the latest one and invalidate the iterator. If the super version changed, recreated the whole iterator. This can help users reuse the iterator more easily.
Closes https://github.com/facebook/rocksdb/pull/2621
Differential Revision: D5464500
Pulled By: siying
fbshipit-source-id: f548bd35e85c1efca2ea69273802f6704eba6ba9
Summary:
The previous implementation of caching `file_size` index made no sense. It only remembered the original span of locked files starting from beginning of `file_size`. We should remember the index after all compactions that have been considered but rejected. This will reduce the work we do while holding the db mutex.
Closes https://github.com/facebook/rocksdb/pull/2624
Differential Revision: D5468152
Pulled By: ajkr
fbshipit-source-id: ab92a4bffe76f9f174d861bb5812b974d1013400
Summary:
This reverts the previous commit 1d7048c598, which broke the build.
Did a `git revert 1d7048c`.
Closes https://github.com/facebook/rocksdb/pull/2627
Differential Revision: D5476473
Pulled By: sagar0
fbshipit-source-id: 4756ff5c0dfc88c17eceb00e02c36176de728d06
Summary: This uses `clang-tidy` to comment out unused parameters (in functions, methods and lambdas) in fbcode. Cases that the tool failed to handle are fixed manually.
Reviewed By: igorsugak
Differential Revision: D5454343
fbshipit-source-id: 5dee339b4334e25e963891b519a5aa81fbf627b2
Summary:
Major changes in this PR:
* Implement CassandraCompactionFilter to remove expired columns and rows (if all column expired)
* Move cassandra related code from utilities/merge_operators/cassandra to utilities/cassandra/*
* Switch to use shared_ptr<> from uniqu_ptr for Column membership management in RowValue. Since columns do have multiple owners in Merge and GC process, use shared_ptr helps make RowValue immutable.
* Rename cassandra_merge_test to cassandra_functional_test and add two TTL compaction related tests there.
Closes https://github.com/facebook/rocksdb/pull/2588
Differential Revision: D5430010
Pulled By: wpc
fbshipit-source-id: 9566c21e06de17491d486a68c70f52d501f27687
Summary:
Seems the only function of the script is to create a new branch, which can be done easily. I'm removing it.
Closes https://github.com/facebook/rocksdb/pull/2623
Differential Revision: D5468681
Pulled By: yiwu-arbug
fbshipit-source-id: 87dea5ecc4c85e06941ccbc36993f7f589063878
Summary:
Remove some of the per-key logging by blob db to reduce noise.
Closes https://github.com/facebook/rocksdb/pull/2587
Differential Revision: D5429115
Pulled By: yiwu-arbug
fbshipit-source-id: b89328282fb8b3c64923ce48738c16017ce7feaf
Summary:
In this test we are deleting 100 files, and we are expecting DeleteScheduler to delete 26 files in the background and 74 files immediately in the foreground
The main purpose of the test is to make sure that we delete files in foreground thread, which is verified in line 546
But sometimes we may end up with 26 files or 25 files in the trash directory because the background thread may be slow and not be able to delete the first file fast enough, so sometimes this test fail.
Remove
```
ASSERT_EQ(CountFilesInDir(trash_dir_), 25);
```
Since it does not have any benefit any way
Closes https://github.com/facebook/rocksdb/pull/2618
Differential Revision: D5458674
Pulled By: IslamAbdelRahman
fbshipit-source-id: 5556a9edfa049db71dce80b8e6ae0fdd25e1e74e
Summary:
This diff addresses two problems. Both problems cause us to miss scheduling desirable compactions. One side effect is compaction picking can spam logs, as there's no delay after failed attempts to pick compactions.
1. If a compaction pulled in a locked input-level file due to user-key overlap, we would not consider picking another file from the same input level.
2. If a compaction pulled in a locked output-level file due to user-key overlap, we would not consider picking any other compaction on any level.
The code changes are dependent, which is why I solved both problems in a single diff.
- Moved input-level `ExpandInputsToCleanCut` into the loop inside `PickFileToCompact`. This gives two benefits: (1) if it fails, we will try the next-largest file on the same input level; (2) we get the fully-expanded input-level key-range with which we can check for pending compactions in output level.
- Added another call to `ExpandInputsToCleanCut` inside `PickFileToCompact`'s to check for compaction conflicts in output level.
- Deleted call to `IsRangeInCompaction` in `PickFileToCompact`, as `ExpandInputsToCleanCut` also correctly handles the case where original output-level files (i.e., ones not pulled in due to user-key overlap) are pending compaction.
Closes https://github.com/facebook/rocksdb/pull/2615
Differential Revision: D5454643
Pulled By: ajkr
fbshipit-source-id: ea3fb5477d83e97148951af3fd4558d2039e9872
Summary:
I decided not even to keep it as an INFO-level log as it is too normal for compactions to be skipped due to locked input files. Removing logging here makes us consistent with how we treat locked files that weren't pulled in due to overlap.
We may want some error handling on line 422, which should never happen when called by `LevelCompactionBuilder::PickCompaction`, as `SetupInitialFiles` skips compactions where overlap causes the output level to pull in locked files.
Closes https://github.com/facebook/rocksdb/pull/2617
Differential Revision: D5458502
Pulled By: ajkr
fbshipit-source-id: c2e5f867c0a77c1812ce4242ab3e085b3eee0bae
Summary:
We don't need this format compatibility hack anymore. We should remove it to make things simpler.
Closes https://github.com/facebook/rocksdb/pull/2607
Differential Revision: D5444107
Pulled By: siying
fbshipit-source-id: 7ef587dd0cacfc15a4083a137adba8e6bfddac7e
Summary:
For forward_compatible_checkout_objs the local branch is already created in previous step. This patch avoid recreating it. This should address "fatal: A branch named '3.10.fb' already exists." errors.
Closes https://github.com/facebook/rocksdb/pull/2606
Differential Revision: D5443786
Pulled By: maysamyabandeh
fbshipit-source-id: 69d5a67b87677429cf36e3a467bd114d341f3b9c
Summary:
The new local branch specified with -b cannot be called master. Use tmp prefix to avoid name collision.
Closes https://github.com/facebook/rocksdb/pull/2600
Differential Revision: D5442944
Pulled By: maysamyabandeh
fbshipit-source-id: 4a623d9b21d6cc01bee812b2799790315bdf5f6e
Summary:
This will fix the error: "error: pathspec '2.2.fb.branch' did not match any file(s) known to git."
Tested by manually sshing to sandcastle and running the command.
Closes https://github.com/facebook/rocksdb/pull/2599
Differential Revision: D5441130
Pulled By: maysamyabandeh
fbshipit-source-id: a22fd6a52221471bafbba8990394b499535e5812
Summary:
Unfortunately we can't use -DNDEBUG yet since we don't properly exclude the test libraries/executables from the non-debug builds on non-MSVC platforms. Previously this was failing on Linux for every build type except `CMAKE_BUILD_TYPE=Debug`.
Reverts a48a62d
Closes https://github.com/facebook/rocksdb/pull/2595
Differential Revision: D5436182
Pulled By: ajkr
fbshipit-source-id: 062f07cc9ce06a073b66054722b27bac1890dca3
Summary:
This patch enables using PinnableSlice for RowCache, changes include
not releasing the cache handle immediately after lookup in TableCache::Get, instead pass a Cleanble function which does Cache::RleaseHandle.
Closes https://github.com/facebook/rocksdb/pull/2492
Differential Revision: D5316216
Pulled By: maysamyabandeh
fbshipit-source-id: d2a684bd7e4ba73772f762e58a82b5f4fbd5d362
Summary:
Fix column_family_test with LITE build. I need this patch to fix 5.6 branch.
Closes https://github.com/facebook/rocksdb/pull/2597
Differential Revision: D5437171
Pulled By: yiwu-arbug
fbshipit-source-id: 88b9dc5925a6b47af10c1b41bc5b07c4251a84b5
Summary:
In gcc-7 the following is an error identified by -Werror=class-memaccess
In file included from ./table/get_context.h:14:0,
from db/version_set.cc:43:
./table/block.h: In constructor ‘rocksdb::BlockReadAmpBitmap::BlockReadAmpBitmap(size_t, size_t, rocksdb::Statistics*)’:
./table/block.h:73:53: error: ‘void* memset(void*, int, size_t)’ clearing an object of type ‘struct std::atomic<unsigned int>’ with no trivial copy-assignment; use value-initialization instead [-Werror=class-memaccess]
memset(bitmap_, 0, bitmap_size * kBytesPersEntry);
^
In file included from ./db/version_set.h:23:0,
from db/version_set.cc:12:
/toolchain/include/c++/8.0.0/atomic:684:12: note: ‘struct std::atomic<unsigned int>’ declared here
struct atomic<unsigned int> : __atomic_base<unsigned int>
^~~~~~~~~~~~~~~~~~~~
As a solution the default initializer can be applied in list context.
Signed-off-by: Daniel Black <daniel.black@au.ibm.com>
Closes https://github.com/facebook/rocksdb/pull/2561
Differential Revision: D5398714
Pulled By: siying
fbshipit-source-id: d883fb88ec7535eee60d551038fe91f14488be36
Summary:
Instead of hard coding the path of the internal repo.
Make TARGETS file work anywhere in fbcode
Closes https://github.com/facebook/rocksdb/pull/2586
Differential Revision: D5428122
Pulled By: IslamAbdelRahman
fbshipit-source-id: 21adec82bfbff14ea93532bee789b5f5bbee5b01
Summary:
We have FLAGS_benchmark_write_rate_limit to limit write rate in
db_bench, but it was not in use for updaterandom benchmark.
Closes https://github.com/facebook/rocksdb/pull/2578
Differential Revision: D5420328
Pulled By: yiwu-arbug
fbshipit-source-id: 5fa48c2b88f2f2dc83d615cb9c40c472bc916835
Summary:
Update blob db to use the newer ROCKS_LOG_* macro.
Closes https://github.com/facebook/rocksdb/pull/2574
Differential Revision: D5414526
Pulled By: yiwu-arbug
fbshipit-source-id: e428753aa5917e8b435cead2db26df586e5d1def
Summary:
We has to remove this line because previously it is only called when use_os_buffer = false. But now we have direct io to replace it.
Closes https://github.com/facebook/rocksdb/pull/2573
Differential Revision: D5412824
Pulled By: yiwu-arbug
fbshipit-source-id: 81f3f0cdf94566bfc09ef2ff123e40cddbe36b36
Summary:
Blob db use StackableDB::get which only get out the
value offset, but not the value.
Fix by making BlobDB::Get override the designated getter.
Closes https://github.com/facebook/rocksdb/pull/2553
Differential Revision: D5396823
Pulled By: yiwu-arbug
fbshipit-source-id: 5a7d1cf77ee44490f836a6537225955382296878
Summary:
this modify allows third-party tables able to support delete range
Closes https://github.com/facebook/rocksdb/pull/2035
Differential Revision: D5407973
Pulled By: ajkr
fbshipit-source-id: 82e364b7dd5a198660788d59543f15b8f95cc418
Summary:
Set to "-Xlint:deprecation -Xlint:unchecked" for non-zero
DEBUG_LEVEL
Shows up a few warnings for later perusal however passes ok.
Closes https://github.com/facebook/rocksdb/pull/2557
Differential Revision: D5408104
Pulled By: sagar0
fbshipit-source-id: 02908602e09b46d33b0dfc0ffd4b11954db03c53
Summary:
The casting seemed to cause a problem.
I think this might increase it to unsigned long.
Closes https://github.com/facebook/rocksdb/pull/2562
Differential Revision: D5406842
Pulled By: siying
fbshipit-source-id: 736adef31448229a58a1a48bdbe77792f36736e8
Summary:
* Create info log before db open to make blob db able to log to LOG file.
* Properly destroy blob db.
Closes https://github.com/facebook/rocksdb/pull/2567
Differential Revision: D5400034
Pulled By: yiwu-arbug
fbshipit-source-id: a49cfaf4b5c67d42d4cbb872bd5a9441828c17ce
Summary:
Found by gcc warning:
x86_64-pc-linux-gnu-g++ --version
x86_64-pc-linux-gnu-g++ (GCC) 7.1.1 20170710
tools/db_bench_tool.cc: In member function 'void rocksdb::Benchmark::RandomWithVerify(rocksdb::ThreadState*)':
tools/db_bench_tool.cc:4430:8: error: '%lu' directive output may be truncated writing between 1 and 19 bytes into a region of size between 0 and 66 [-Werror=format-truncation=]
void RandomWithVerify(ThreadState* thread) {
^~~~~~~~~~~~~~~~
tools/db_bench_tool.cc:4430:8: note: directive argument in the range [0, 9223372036854775807]
tools/db_bench_tool.cc:4492:13: note: 'snprintf' output between 37 and 128 bytes into a destination of size 100
snprintf(msg, sizeof(msg),
~~~~~~~~^~~~~~~~~~~~~~~~~~
"( get:%" PRIu64 " put:%" PRIu64 " del:%" PRIu64 " total:%" \
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
PRIu64 " found:%" PRIu64 ")",
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
gets_done, puts_done, deletes_done, readwrites_, found);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
Makefile:1707: recipe for target 'tools/db_bench_tool.o' failed
Closes https://github.com/facebook/rocksdb/pull/2558
Differential Revision: D5398703
Pulled By: siying
fbshipit-source-id: 6ffa552bbd8b59cfc2c36289f86ff9b9acca8ca6
Summary:
Instead of ignoring UBSan checks, fix the negative shifts in
Hash(). Also add test to make sure the hash values are stable over
time. The values were computed before this change, so the test also
verifies the correctness of the change.
Closes https://github.com/facebook/rocksdb/pull/2546
Differential Revision: D5386369
Pulled By: yiwu-arbug
fbshipit-source-id: 6de4b44461a544d6222cc5d72d8cda2c0373d17e
Islam AbdelRahman