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

Only run secondary db verification for blackbox crash tests #13341

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

archang19
Copy link
Contributor

@archang19 archang19 commented Jan 28, 2025

Summary

This is a continuation of #13338, which aims to address crash test failures caused by #13281.

I looked through the test failures again, and they all are for the whitebox tests.

I am pretty sure this is because the whitebox tests have 20 for reopen whereas the blackbox tests have 0 inside whitebox_default_params and blackbox_default_params.

The error logs for the ASAN and TSAN failures point to the ReOpen method, so I think that if we just avoid this code path, we can avoid the failures.

I do not think that the whitebox tests give us additional information for the sole purpose of secondary DB verification compared to the blackbox tests. Thus, I think it is likely better to just enable secondary DB verification on blackbox tests only. We still will want to monitor for data verification failures and see no verification failures for an extended period of time.

Test Plan

Monitor recurring crash test runs.

I confirmed the simple blackbox test can give 1 for test_secondary.

python3 tools/db_crashtest.py --simple blackbox 

whereas test_secondary is not even specified in

python3 tools/db_crashtest.py --simple whitebox 

@archang19 archang19 marked this pull request as ready for review January 28, 2025 19:13
@archang19 archang19 requested a review from anand1976 January 28, 2025 19:13
@facebook-github-bot
Copy link
Contributor

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

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.

2 participants