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.

Sign up for GitHub

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

Jump to bottom

test: Add fault injection in cache fuzzer #11969

Closed
wants to merge 1 commit into from
Closed

test: Add fault injection in cache fuzzer #11969

wants to merge 1 commit into from
+45 −6

Conversation

zacw7
Copy link
Contributor

@zacw7 zacw7 commented Dec 27, 2024

Differential Revision: D67662693

@zacw7 zacw7 requested a review from majetideepak as a code owner December 27, 2024 02:02
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 27, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67662693

@netlify Netlify
Copy link

netlify bot commented Dec 27, 2024 •
edited
Loading

✅ Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 5441501
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/67703d5d4a58160008b50070

xiaoxmeng
xiaoxmeng reviewed Dec 27, 2024
View reviewed changes
Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@zacw7 LGTM % minors. Thanks!

velox/exec/fuzzer/CacheFuzzer.cpp Outdated
if (FLAGS_enable_file_faulty_injection) {
faultyFileSystem()->setFileInjectionHook([&](FaultFileOperation* op) {
if (std::regex_match(op->path, kDataFileNamePattern)) {
// Skip errors on input data files.
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the input data files here?

I think you could capture the input file directory here and match as prefix? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The input files are in the same directory, named as file_{} while cache files are named as cache_{}.

velox/exec/fuzzer/CacheFuzzer.cpp Outdated
dist(rd) <= kFileWriteErrorRate) {
VELOX_FAIL("Inject hook write failure");
}
if (op->type == FaultFileOperation::Type::kReadv &&
Copy link
Contributor

Choose a reason for hiding this comment

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

ssd cache all use readv? Thanks!

Copy link
Contributor Author

@zacw7 zacw7 Dec 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

actually evictlog read uses read and checkpoint write uses append. let me add those cases to the injection.

velox/exec/fuzzer/CacheFuzzer.cpp Outdated Show resolved Hide resolved
velox/exec/fuzzer/CacheFuzzer.cpp Outdated Show resolved Hide resolved
zacw7 added a commit to zacw7/velox that referenced this pull request Dec 27, 2024
@zacw7 @facebook-github-bot
test: Add fault injection in cache fuzzer (facebookincubator#11969)
2c461b6 
Summary: Pull Request resolved: facebookincubator#11969

Differential Revision: D67662693
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67662693

zacw7 added a commit to zacw7/velox that referenced this pull request Dec 27, 2024
@zacw7 @facebook-github-bot
test: Add fault injection in cache fuzzer (facebookincubator#11969)
41507b5 
Summary: Pull Request resolved: facebookincubator#11969

Differential Revision: D67662693
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67662693

@zacw7 zacw7 requested a review from xiaoxmeng December 27, 2024 18:31
zacw7 added a commit to zacw7/velox that referenced this pull request Dec 27, 2024
@zacw7 @facebook-github-bot
test: Add fault injection in cache fuzzer (facebookincubator#11969)
b23a86d 
Summary: Pull Request resolved: facebookincubator#11969

Differential Revision: D67662693
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67662693

zacw7 added a commit to zacw7/velox that referenced this pull request Dec 27, 2024
@zacw7 @facebook-github-bot
test: Add fault injection in cache fuzzer (facebookincubator#11969)
1e9374a 
Summary: Pull Request resolved: facebookincubator#11969

Differential Revision: D67662693
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67662693

xiaoxmeng
xiaoxmeng approved these changes Dec 28, 2024
View reviewed changes
Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@zacw7 LGTM. Thanks!

velox/exec/fuzzer/CacheFuzzer.cpp Outdated Show resolved Hide resolved
zacw7 added a commit to zacw7/velox that referenced this pull request Dec 28, 2024
@zacw7 @facebook-github-bot
test: Add fault injection in cache fuzzer (facebookincubator#11969)
d98de32 
Summary: Pull Request resolved: facebookincubator#11969

Reviewed By: xiaoxmeng

Differential Revision: D67662693
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67662693

@zacw7 @facebook-github-bot
test: Add fault injection in cache fuzzer (facebookincubator#11969)
5441501 
Summary: Pull Request resolved: facebookincubator#11969

Reviewed By: xiaoxmeng

Differential Revision: D67662693
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67662693

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 20eb8ec.

@zacw7 zacw7 deleted the export-D67662693 branch December 29, 2024 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiaoxmeng xiaoxmeng xiaoxmeng approved these changes

@majetideepak majetideepak Awaiting requested review from majetideepak majetideepak is a code owner

@kewang1024 kewang1024 Awaiting requested review from kewang1024

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zacw7 @facebook-github-bot @xiaoxmeng