Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General bug fixes #4

Closed

Conversation

Yuval-Ariel
Copy link
Contributor

No description provided.

@Yuval-Ariel Yuval-Ariel requested a review from isaac-io June 6, 2022 12:08
@Yuval-Ariel Yuval-Ariel self-assigned this Jun 6, 2022
isaac-io and others added 10 commits June 12, 2022 16:36
RocksDB currently fails to build with clang 12 due to newly added
diagnostics. This is problematic for fuzz testing in CI and for developers
building locally with clang, and thankfully these are easily fixable.
Currently the CMake config only looks for ccache, so developers using
sccache need to manually set the launch commands. Add a check for sccache
to spare them the trouble.

While at it, clean up the CMake config and remove ccache as the link launch
wrapper since it will just forward to the underlying linker anyway, and
also regenerate the buck TARGETS file (we might want to drop support for
it at some point, but for now let's keep it consistent).
This commit originally imported fuzzing tools from rocksdb.
However, after the rebase on 6.21 (from 6.11.4) only the compilation
fixes are left.

Original commit message:

Building them is a bit of a pain, but the OSS-Fuzz build scripts are
a good reference on how to do that:
https://github.com/google/oss-fuzz/tree/master/projects/rocksdb

This commit also fixes some clang compilation errors.
Yuval-Ariel and others added 6 commits June 20, 2022 15:16
set opened_successfully_ after successfully opening db in
db_impl_readonly and in db_impl_secondary.

changes were made according to db_logical_block_size_cache_test and db_secondary_test.
…ence of user comparators

This will create a memtable bloom filter if the user happens to also set
memtable_prefix_bloom_size_ratio to a value greater than 0.0, which is
incompatible with user comparators that don't consider the entire key
representation as is.

While we're at it, add a validation for filter policies in Block Based
table in conjunction with a user comparator that may consider different
byte contents being equal, and also check for memtable_whole_key_filtering.
SPDB-197 introduced a stricter check for custom comparators, and that
apparently broke test_util's custom comparators which don't override the
CanKeysWithDifferentByteContentsBeEqual() method, and which by default
returns true, which in turn causes these comparators to triggers the
stricter check even though they don't, in fact, consider keys with different
byte contents equal.

This is easily fixed by overriding this method to return false as it should.
After SPDB-197, custom comparators that don't return false for the check
CanKeysWithDifferentByteContentsBeEqual() error out on DB open due to the
automatic addition of a filter policy on our part. However, most of these
tests indeed never compare keys with different byte contents as equals, so
the easy fix is to just override CanKeysWithDifferentByteContentsBeEqual()
to return false.

The tests that are failing because they do compare keys with different
byte contents as equal need to override their filter policy, as done in
SPDB-197.

While at it, also fix an embarrassing bug in the filter policy implementation
done as part of SPDB-197.
@Yuval-Ariel Yuval-Ariel force-pushed the yuval/722-infrast-bug_fixes branch from 2629ce9 to f18262b Compare June 20, 2022 12:20
fix both these tests to operate not only on the default cf
@Yuval-Ariel Yuval-Ariel force-pushed the yuval/722-infrast-bug_fixes branch from f18262b to cf4ff2c Compare June 20, 2022 16:37
@isaac-io isaac-io mentioned this pull request Jul 20, 2022
udi-speedb added a commit that referenced this pull request Aug 25, 2022
udi-speedb added a commit that referenced this pull request Aug 25, 2022
udi-speedb added a commit that referenced this pull request Sep 5, 2022
udi-speedb added a commit that referenced this pull request Sep 6, 2022
udi-speedb added a commit that referenced this pull request Dec 7, 2022
udi-speedb added a commit that referenced this pull request Dec 7, 2022
Yuval-Ariel added a commit that referenced this pull request May 2, 2023
@Yuval-Ariel Yuval-Ariel deleted the yuval/722-infrast-bug_fixes branch May 11, 2023 08:46
Yuval-Ariel added a commit that referenced this pull request Jun 12, 2023
Yuval-Ariel added a commit that referenced this pull request Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants