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

8 1 1 rebase main candidate #490

Closed
wants to merge 201 commits into from
Closed

Conversation

Yuval-Ariel
Copy link
Contributor

closes #478

rtpro and others added 30 commits April 24, 2023 15:41
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.
…Data()

When there's no data in the buffer, there's nothing to drop anyway, and
providing 0 to the rand->Uniform() function results in a division by zero
error (SIGFPE), so just check and do nothing in that case.
When fixing the clang compilation errors (offsetof() being applied
to non standard layout types), the pointer flags were mistakenly set
to `kNone` from `kAllowNone`. This didn't cause an issue with the tests
on 6.22.1, but it does after the rebase on 7.2.2. Add the missing flags
back so that the test passes.
This helps debug things by correlating with other events in the system,
which we are unable to connect currently due to lack of information on
which process failed and when.
SPDB-225 added copying of the database during tests in order to preserve
the state of the database, but after a successful run there's no reason
to keep these copies around, so remove them.
The command is printed anyway by execute_cmd(), so there's no need to
print it in whitebox_crash_main() as well.
The current code is broken, because it effectively sets bool() as the
conversion function for boolean arguments, but bool() returns True for
every non-empty string, so that doesn't work.

Add a converter function to parse boolean value and set it as the type
for argparse in case the argument is of type bool.
…snapshot

In batched snapshot mode each key is duplicated 10 times and prefixed with
an ASCII digit in the range 0-9. However, the snapshot verification code
assumed that each key is present exactly as generated and exactly once,
so the calculated key index was bogus and lead to an invalid memory access.

Fix it by making the snapshot verification code aware of the batched mode
and apply the key conversion and verification accordingly.

While at it, clean up the way the verification bit vector is allocated.
in addition, add randomize_operation_type_percentages
This changes the way the stress test parameters are generated in order to
increase the coverage of the test and check for more edge cases.

Additionally, this change adds the option to run whitebox tests with kill
points disabled for testing functionality without crash recovery.

SPDB-390: crash_test: fix bool flags

SPDB-388: crash_test: disallow zero key dist val

zero vals in key_len_percent_dist are incompatible with
db_stress_common.h::GetIntVal
…ncy is on

running with both options, the stress test chosen is cf_consistency_stress
which does not use '0' - '9' as prefix for keys as in batched_ops_stress.
when doing compare_full_db_state_snapshot, the code expects each key to
have the prefix since test_batches_snapshots==true.
there is no point when both flags are on since each is run as a different stress test.
also fix handling of customopspercent which atm is only applicable to
test_multiops_txn by making sure its 0 in all the other configs.
ofriedma and others added 12 commits May 4, 2023 09:29
Currently even if objects have been expired we still return them until
compaction delete those objects.

This commit adds a skip_expired_data to the ReadOptions, if this option
is true, it Get, MultiGet and iterator will not return objects that have been
expired by the configured ttl.
Currently this feature does not include support for Merge operations.
For Read Only and iterator pinned data the skip_expired_data will be
enforced and expired keys will not be returned.

Resolves: #394, #417

Pull Request: #403
beezcli is an interactive tool wrapping ldb

This commit adds some enhancement to ldb

Resolves: #407
* added readline-devel package to the artifact-release.yml, needed by beezcli
This PR adds the ability to create a Filter (FilterPolicy) in Java via the CreateFromString methodology. This change allows the other filter policies (such as Ribbon Filter and plugin filters) to be created in Java using the C++ infrastructure.
which was accidentally removed in
general: replace RocksDB references in strings with Speedb (#64)
as part of - Speedb's Paired Block Bloom (#29)
bloom filter stress testing

DBIOFailureTest.DropWrites fails rarely
@Yuval-Ariel Yuval-Ariel requested a review from ayulas May 4, 2023 08:21
@Yuval-Ariel Yuval-Ariel self-assigned this May 4, 2023
Copy link
Contributor

@ayulas ayulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i went over the changes files. i hope i didnt missed something . i have few comments

CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
db/db_impl/db_impl_compaction_flush.cc Show resolved Hide resolved
Copy link
Contributor

@ayulas ayulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that issues I raised are not related to the rebase and a separate tickets will be opened

Copy link
Contributor

@ayulas ayulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that issues I raised are not related to the rebase and a separate tickets will be opened

Copy link
Contributor

@mrambacher mrambacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please enter an issue for the tests that are disabled as part of this upgrade.

@Yuval-Ariel
Copy link
Contributor Author

Please enter an issue for the tests that are disabled as part of this upgrade.

there already is -
#491
#492
#493
#499

@Yuval-Ariel
Copy link
Contributor Author

done

@Yuval-Ariel Yuval-Ariel deleted the 8-1-1-rebase-main-candidate branch May 11, 2023 08:21
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.