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

Add faulty file system for io failure injections #9457

Closed
wants to merge 1 commit into from

Conversation

xiaoxmeng
Copy link
Contributor

@xiaoxmeng xiaoxmeng commented Apr 11, 2024

Add faulty file system to allow us to inject failure for low-level file operations in unit and fuzzer test.
It is implemented under velox filesystem. It is wrapper on top of the real file system, it either inject
errors or delegate the file operations to the underlying file system. We extend TempDirectoryPath or
TempFilePath to allow user to specify whether enable fault injection or not in test. If it does, it returns
the file or directory path with faulty file system scheme prefix (faulty:). This allows the velox file system
to open the file through faulty file system and by removing the faulty fs scheme prefix, it can delegate
the file operation to the corresponding underlying file system.

We support three kinds of file fault injections: (1) error injection which throws for a set (or all) file operation
types; (2) delay injection for a set (or all) file operation types; (3) custom injection with user provided fault
injection hook. We define the base structure FaultFileOperation to capture the file fault injection parameters
and each file api will extend with its own operation type.

This PR only supports fault injection for read file operations. Next we will extend to other file operation types
as well as fs operation types.

@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 Apr 11, 2024
Copy link

netlify bot commented Apr 11, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit e1a431d
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/661c2742904e160008bff36e

@xiaoxmeng xiaoxmeng force-pushed the scan branch 8 times, most recently from 57630af to 544c6d9 Compare April 12, 2024 20:21
@xiaoxmeng xiaoxmeng marked this pull request as ready for review April 12, 2024 20:30
@xiaoxmeng xiaoxmeng changed the title [WIP]Add faulty file system for io failure injection Add faulty file system for io failure injections Apr 12, 2024
@facebook-github-bot
Copy link
Contributor

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

velox/common/file/tests/utils/FaultyFile.h Outdated Show resolved Hide resolved
velox/common/file/tests/utils/FaultyFile.h Outdated Show resolved Hide resolved
velox/common/file/tests/utils/FaultyFile.h Outdated Show resolved Hide resolved
velox/common/file/tests/utils/FaultyFile.h Outdated Show resolved Hide resolved
velox/common/file/tests/utils/FaultyFileSystem.cpp Outdated Show resolved Hide resolved
velox/common/file/tests/utils/FaultyFile.cpp Outdated Show resolved Hide resolved
Comment on lines 74 to 75
/// 'opTypes' is empty, it injects error for all kinds of file operation
/// types.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we want to be explicit here. I could be easy to get confused. If none is provided, should we just not inject? Can we add Type::kAll to indicate that we want to inject to all operations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is common to treat an empty set to match all. User will only call this method to inject fault.

Comment on lines 91 to 93
// Defines the file injection setup and only one type of injection can be set
// at a time.
struct FileInjections {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean either delay type or exception type can be set? If so can we make members const? Otherwise direct access to members can make this struct in an inconsistent state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a default case used to copy the injection settings. I will add some sanity check on the execution path. This is a private structure used internally.

@xiaoxmeng xiaoxmeng force-pushed the scan branch 3 times, most recently from 49bd2c3 to 98a300c Compare April 13, 2024 17:31
@facebook-github-bot
Copy link
Contributor

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

@xiaoxmeng xiaoxmeng force-pushed the scan branch 2 times, most recently from a51d498 to 21f5227 Compare April 13, 2024 17:34
@facebook-github-bot
Copy link
Contributor

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

@xiaoxmeng xiaoxmeng force-pushed the scan branch 3 times, most recently from b043084 to 3af7411 Compare April 13, 2024 18:44
@facebook-github-bot
Copy link
Contributor

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

xiaoxmeng added a commit to xiaoxmeng/velox that referenced this pull request Apr 14, 2024
)

Summary:
Add faulty file system to allow us to inject failure for low-level file operations in unit and fuzzer test.
It is implemented under velox filesystem. It is wrapper on top of the real file system, it either inject
errors or delegate the file operations to the underlying file system. We extend TempDirectoryPath or
TempFilePath to allow user to specify whether enable fault injection or not in test. If it does, it returns
the file or directory path with faulty file system scheme prefix (faulty:). This allows the velox file system
to open the file through faulty file system and by removing the faulty fs scheme prefix, it can delegate
the file operation to the corresponding underlying file system.

We support three kinds of file fault injections: (1) error injection which throws for a set (or all) file operation
types; (2) delay injection for a set (or all) file operation types; (3) custom injection with user provided fault
injection hook. We define the base structure FaultFileOperation to capture the file fault injection parameters
and each file api will extend with its own operation type.

This PR only supports fault injection for read file operations. Next we will extend to other file operation types
as well as fs operation types.

Pull Request resolved: facebookincubator#9457

Reviewed By: tanjialiang, oerling

Differential Revision: D56079500

Pulled By: xiaoxmeng
xiaoxmeng added a commit to xiaoxmeng/velox that referenced this pull request Apr 14, 2024
)

Summary:
Add faulty file system to allow us to inject failure for low-level file operations in unit and fuzzer test.
It is implemented under velox filesystem. It is wrapper on top of the real file system, it either inject
errors or delegate the file operations to the underlying file system. We extend TempDirectoryPath or
TempFilePath to allow user to specify whether enable fault injection or not in test. If it does, it returns
the file or directory path with faulty file system scheme prefix (faulty:). This allows the velox file system
to open the file through faulty file system and by removing the faulty fs scheme prefix, it can delegate
the file operation to the corresponding underlying file system.

We support three kinds of file fault injections: (1) error injection which throws for a set (or all) file operation
types; (2) delay injection for a set (or all) file operation types; (3) custom injection with user provided fault
injection hook. We define the base structure FaultFileOperation to capture the file fault injection parameters
and each file api will extend with its own operation type.

This PR only supports fault injection for read file operations. Next we will extend to other file operation types
as well as fs operation types.

Pull Request resolved: facebookincubator#9457

Reviewed By: tanjialiang, oerling

Differential Revision: D56079500

Pulled By: xiaoxmeng
@facebook-github-bot
Copy link
Contributor

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

xiaoxmeng added a commit to xiaoxmeng/velox that referenced this pull request Apr 14, 2024
)

Summary:
Add faulty file system to allow us to inject failure for low-level file operations in unit and fuzzer test.
It is implemented under velox filesystem. It is wrapper on top of the real file system, it either inject
errors or delegate the file operations to the underlying file system. We extend TempDirectoryPath or
TempFilePath to allow user to specify whether enable fault injection or not in test. If it does, it returns
the file or directory path with faulty file system scheme prefix (faulty:). This allows the velox file system
to open the file through faulty file system and by removing the faulty fs scheme prefix, it can delegate
the file operation to the corresponding underlying file system.

We support three kinds of file fault injections: (1) error injection which throws for a set (or all) file operation
types; (2) delay injection for a set (or all) file operation types; (3) custom injection with user provided fault
injection hook. We define the base structure FaultFileOperation to capture the file fault injection parameters
and each file api will extend with its own operation type.

This PR only supports fault injection for read file operations. Next we will extend to other file operation types
as well as fs operation types.

Pull Request resolved: facebookincubator#9457

Reviewed By: tanjialiang, oerling

Differential Revision: D56079500

Pulled By: xiaoxmeng
@facebook-github-bot
Copy link
Contributor

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

xiaoxmeng added a commit to xiaoxmeng/velox that referenced this pull request Apr 14, 2024
)

Summary:
Add faulty file system to allow us to inject failure for low-level file operations in unit and fuzzer test.
It is implemented under velox filesystem. It is wrapper on top of the real file system, it either inject
errors or delegate the file operations to the underlying file system. We extend TempDirectoryPath or
TempFilePath to allow user to specify whether enable fault injection or not in test. If it does, it returns
the file or directory path with faulty file system scheme prefix (faulty:). This allows the velox file system
to open the file through faulty file system and by removing the faulty fs scheme prefix, it can delegate
the file operation to the corresponding underlying file system.

We support three kinds of file fault injections: (1) error injection which throws for a set (or all) file operation
types; (2) delay injection for a set (or all) file operation types; (3) custom injection with user provided fault
injection hook. We define the base structure FaultFileOperation to capture the file fault injection parameters
and each file api will extend with its own operation type.

This PR only supports fault injection for read file operations. Next we will extend to other file operation types
as well as fs operation types.

Pull Request resolved: facebookincubator#9457

Reviewed By: tanjialiang, oerling

Differential Revision: D56079500

Pulled By: xiaoxmeng
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

xiaoxmeng added a commit to xiaoxmeng/velox that referenced this pull request Apr 14, 2024
)

Summary:
Add faulty file system to allow us to inject failure for low-level file operations in unit and fuzzer test.
It is implemented under velox filesystem. It is wrapper on top of the real file system, it either inject
errors or delegate the file operations to the underlying file system. We extend TempDirectoryPath or
TempFilePath to allow user to specify whether enable fault injection or not in test. If it does, it returns
the file or directory path with faulty file system scheme prefix (faulty:). This allows the velox file system
to open the file through faulty file system and by removing the faulty fs scheme prefix, it can delegate
the file operation to the corresponding underlying file system.

We support three kinds of file fault injections: (1) error injection which throws for a set (or all) file operation
types; (2) delay injection for a set (or all) file operation types; (3) custom injection with user provided fault
injection hook. We define the base structure FaultFileOperation to capture the file fault injection parameters
and each file api will extend with its own operation type.

This PR only supports fault injection for read file operations. Next we will extend to other file operation types
as well as fs operation types.

Pull Request resolved: facebookincubator#9457

Reviewed By: tanjialiang, oerling

Differential Revision: D56079500

Pulled By: xiaoxmeng
@facebook-github-bot
Copy link
Contributor

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

xiaoxmeng added a commit to xiaoxmeng/velox that referenced this pull request Apr 14, 2024
)

Summary:
Add faulty file system to allow us to inject failure for low-level file operations in unit and fuzzer test.
It is implemented under velox filesystem. It is wrapper on top of the real file system, it either inject
errors or delegate the file operations to the underlying file system. We extend TempDirectoryPath or
TempFilePath to allow user to specify whether enable fault injection or not in test. If it does, it returns
the file or directory path with faulty file system scheme prefix (faulty:). This allows the velox file system
to open the file through faulty file system and by removing the faulty fs scheme prefix, it can delegate
the file operation to the corresponding underlying file system.

We support three kinds of file fault injections: (1) error injection which throws for a set (or all) file operation
types; (2) delay injection for a set (or all) file operation types; (3) custom injection with user provided fault
injection hook. We define the base structure FaultFileOperation to capture the file fault injection parameters
and each file api will extend with its own operation type.

This PR only supports fault injection for read file operations. Next we will extend to other file operation types
as well as fs operation types.

Pull Request resolved: facebookincubator#9457

Reviewed By: tanjialiang, oerling

Differential Revision: D56079500

Pulled By: xiaoxmeng
)

Summary:
Add faulty file system to allow us to inject failure for low-level file operations in unit and fuzzer test.
It is implemented under velox filesystem. It is wrapper on top of the real file system, it either inject
errors or delegate the file operations to the underlying file system. We extend TempDirectoryPath or
TempFilePath to allow user to specify whether enable fault injection or not in test. If it does, it returns
the file or directory path with faulty file system scheme prefix (faulty:). This allows the velox file system
to open the file through faulty file system and by removing the faulty fs scheme prefix, it can delegate
the file operation to the corresponding underlying file system.

We support three kinds of file fault injections: (1) error injection which throws for a set (or all) file operation
types; (2) delay injection for a set (or all) file operation types; (3) custom injection with user provided fault
injection hook. We define the base structure FaultFileOperation to capture the file fault injection parameters
and each file api will extend with its own operation type.

This PR only supports fault injection for read file operations. Next we will extend to other file operation types
as well as fs operation types.

Pull Request resolved: facebookincubator#9457

Reviewed By: tanjialiang, oerling

Differential Revision: D56079500

Pulled By: xiaoxmeng
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@xiaoxmeng merged this pull request in d796cfc.

@xiaoxmeng xiaoxmeng deleted the scan branch April 14, 2024 20:30
Copy link

Conbench analyzed the 1 benchmark run on commit d796cfc8.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request Jun 7, 2024
)

Summary:
Add faulty file system to allow us to inject failure for low-level file operations in unit and fuzzer test.
It is implemented under velox filesystem. It is wrapper on top of the real file system, it either inject
errors or delegate the file operations to the underlying file system. We extend TempDirectoryPath or
TempFilePath to allow user to specify whether enable fault injection or not in test. If it does, it returns
the file or directory path with faulty file system scheme prefix (faulty:). This allows the velox file system
to open the file through faulty file system and by removing the faulty fs scheme prefix, it can delegate
the file operation to the corresponding underlying file system.

We support three kinds of file fault injections: (1) error injection which throws for a set (or all) file operation
types; (2) delay injection for a set (or all) file operation types; (3) custom injection with user provided fault
injection hook. We define the base structure FaultFileOperation to capture the file fault injection parameters
and each file api will extend with its own operation type.

This PR only supports fault injection for read file operations. Next we will extend to other file operation types
as well as fs operation types.

Pull Request resolved: facebookincubator#9457

Reviewed By: tanjialiang, oerling

Differential Revision: D56079500

Pulled By: xiaoxmeng

fbshipit-source-id: e7cccbf0b51b3f94c161949b22a88cb560d58b7d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

3 participants