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

cover single level universal in crash test #6818

Closed
wants to merge 1 commit into from

Conversation

ajkr
Copy link
Contributor

@ajkr ajkr commented May 6, 2020

Test Plan:

fast whitebox test and verify there are some single-level universal and
some multi-level universal runs.

$ python ./tools/db_crashtest.py whitebox --simple -max_key=1000000 -value_size_mult=33 -write_buffer_size=524288 -target_file_size_base=524288 -max_bytes_for_level_base=2097152 --duration=120 --interval=10 --ops_per_thread=1000 --random_kill_odd=887

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ajkr ajkr force-pushed the crash-test-single-level-universal branch from 72053ff to 126fddd Compare May 6, 2020 19:21
@@ -186,6 +186,9 @@ DEFINE_int64(compressed_cache_size, -1,
DEFINE_int32(compaction_style, ROCKSDB_NAMESPACE::Options().compaction_style,
"");

DEFINE_int32(num_levels, ROCKSDB_NAMESPACE::Options().num_levels,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add this to db_stress_common.h as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Test Plan:

fast whitebox test and verify there are some single-level universal and
some multi-level universal runs.

```
$ python ./tools/db_crashtest.py whitebox --simple -max_key=1000000 -value_size_mult=33 -write_buffer_size=524288 -target_file_size_base=524288 -max_bytes_for_level_base=2097152 --duration=120 --interval=10 --ops_per_thread=1000 --random_kill_odd=887
```
@ajkr ajkr force-pushed the crash-test-single-level-universal branch from 126fddd to 1d2c622 Compare May 6, 2020 23:56
@facebook-github-bot
Copy link
Contributor

@ajkr has updated the pull request. Re-import the pull request

Copy link
Contributor Author

@ajkr ajkr left a comment

Choose a reason for hiding this comment

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

Thanks for the review.

@@ -186,6 +186,9 @@ DEFINE_int64(compressed_cache_size, -1,
DEFINE_int32(compaction_style, ROCKSDB_NAMESPACE::Options().compaction_style,
"");

DEFINE_int32(num_levels, ROCKSDB_NAMESPACE::Options().num_levels,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@ajkr merged this pull request in 1f20df2.

# it sometimes.
if random.randint(0, 1) == 1:
additional_opts.update({
"num_levels": 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to migrate the data if there is existing data in non-L0 levels for this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh, I assumed that happened automatically, as in H2 of the whitebox crash test, it alternates between universal and FIFO compaction. So I thought it'd have to clear the DB between runs...

Copy link
Contributor Author

@ajkr ajkr May 11, 2020

Choose a reason for hiding this comment

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

How about set destroy_db_initially to true for these cases that may change compaction style? At least until we come up with something better than cycling through the rare compaction styles during the second half of crash test...

facebook-github-bot pushed a commit that referenced this pull request Sep 27, 2022
Summary:
An add-on to #6818 to complete adding single-level universal compaction to stress/crash testing.

Pull Request resolved: #10732

Test Plan:
- Locally run for 10 min `python3 ./tools/db_crashtest.py whitebox --simple --compaction_style=1 --num_levels=1  -max_key=1000000 -value_size_mult=33 -write_buffer_size=524288 -target_file_size_base=524288 -max_bytes_for_level_base=2097152 --duration=120 --interval=10 --ops_per_thread=1000 --random_kill_odd=887`
   - Check LOG to confirm single-level universal compaction is called
- Manual testing and log checking to ensure destroy_db_initially=1 is correctly set across runs with different compaction styles (i.e, in the second half of whitebox testing).
- [ongoing]CI jobs stress test

Reviewed By: ajkr

Differential Revision: D39797612

Pulled By: ajkr

fbshipit-source-id: 16f5c40c3464c57360c06c8305f92118e426149c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants