Add useBaseAddress to OptionTypeInfo #397
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: We have seen some rare crash test failures in HyperClockCache, and the source could certainly be a bug fixed in this change, in ClockHandleTable::ConstApplyToEntriesRange. It wasn't properly accounting for the fact that incrementing the acquire counter could be ineffective, due to parallel updates. (When incrementing the acquire counter is ineffective, it is incorrect to then decrement it.) This change includes some other minor clean-up in HyperClockCache, and adds stats_dump_period_sec with a much lower period to the crash test. This should be the primary caller of ApplyToEntries, in collecting cache entry stats. Pull Request resolved: facebook/rocksdb#10768 Test Plan: haven't been able to reproduce the failure, but should be in a better state (bug fix and improved crash test) Reviewed By: anand1976 Differential Revision: D40034747 Pulled By: anand1976 fbshipit-source-id: a06fcefe146e17ee35001984445cedcf3b63eb68
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.
…mUnsyncedData() 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.
db_stress does not support running successive runs with different values of test_batches_snapshots.
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.
user can now switch between kDataBlockBinarySearch and kDataBlockBinaryAndHash
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
previously approved these changes
Oct 19, 2023
ofriedma
previously approved these changes
Oct 24, 2023
|
@mrambacher, @ofriedma - Please handle this PR so we may merge it. |
|
@Yuval-Ariel ready to be merge |
|
@mrambacher - Please fix the license / History issue. Thanks |
ofriedma
previously approved these changes
Oct 30, 2023
|
@or - Please review, resolve applicable comments, and indicate when ready to merge. Thanks |
ofriedma
approved these changes
Oct 31, 2023
udi-speedb
pushed a commit
that referenced
this pull request
Nov 23, 2023
udi-speedb
pushed a commit
that referenced
this pull request
Dec 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(This replaces #308, which will be abandoned due to git branching issues).
When an OptionsTypeInfo is defined, it is passed the offset of the field in question. This offset is the relative location of the field in the structure being analyzed. This PR adds a "useBaseAddress" flag that tells the option system to use the base address rather than the offset to any custom functions. The flag is only used if a custom function is used and is ignored if using the default behaviors.
This new flag is useful when there are relationships between fields in the structure. For example:
If a std::shared_ptr shared_env value is added to the DBOptions, the "shared" and "env" can be marked as useBase. This would allow the system to serialize either of the values (either shared or env and not both) and allow the Create method to update the other value.
For things like Compaction, the different Options (CompactionOptionsFIFO/Universal) could be skipped during serialization if they did not apply (so if FIFO was specified, do not store or validate Universal options).
Fields that are specific to certain implementations could be moved/associated with the implementation.
When multiple fields are validated/prepared/sanitized together, the code can be placed in the OptionsTypeInfo rather than writing class-specific implementations of these methods. For example, the BlockBasedTableOptions have fields that are related. Using this new flag allows the relationship to be initialized and validated without the class methods.