-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
Create InjectionFileSystem/NoSyncFileSystem classes #9545
base: main
Are you sure you want to change the base?
Conversation
InjectionFileSystem is a base FileSystem that allows extensions to intercept calls to the File APIs (like Read, Append, Sync, etc). This class allows those methods to be intercepted without writing new classes for all of the File (Sequential, Random, etc) classes. NoSyncFileSystem is an implementation of an InjectionFileSystem that intercepts and optionally skips the various sync (Sync, FSync, RangeSync, Directory FSync) calls.
@mrambacher has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding NoSyncFileSystem
, my opinion remains the same as #9356 (comment).
The ramdisk approach workaround for the slow F_FULLFSYNC
on OSX should suffice. One limitation is that, after running the tests make check
several times, the ramdisk may get full because some of the unit tests do not cleanup after completion, and you need to manually clean it up, which imho, is not a problem. I remember you also submitted #9539 to cleanup the unit tests directories, and I think this is the right direction if you think periodic manual cleaning up is intolerable.
The ramdisk does not work so well on the mac. First, there is the problem that the tests do not clean up after themselves. The other problem is to run in parallel mode, even from a clean disk, will sometimes fail as some tests use a lot of space, exhausting the ramdisk. |
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
I might also add that the code for the NoSyncFileSystem is effectively copied from the SpecialEnv. A later PR can create a SpecialFileSystem that extends this new class. This will allow the classes who wish to use Special and those who do not to start from the same/similar Env/FileSystem classes. |
Then this is the number 1 thing which I think we should fix. |
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
The InjectionFileSystem is a base/abstract class that captures the IO methods associated with the FSxxxFile classes and allows the FileSystem to do something with them. For example, the CountedFileSystem can count the number of operations and bytes and the NoSyncFileSystem can skip the various Sync operations. The FaultInjectionFileSystem can insert the appropriate failures into the IO path.
The InjectionFileSystem is meant to make it easier to intercept these IO operations in a more standard and coherent way. Previously, every type of InjectionFileSystem would need to write their own FSxxxFile extension classes for each type/operation that is extended. This methodology leads to cases where some of the operations were missed in some implementations.
Note that the FaultInjectionFileSystem is not fully "Injection" ready, as it still has its own TestWritableFile class. This implementation will be cleaned up in a later PR. Additionally a later PR will make the SpecialEnv into a SpecialFileSystem taking advantage of these capabilities.
The NoSyncFileSystem is used for some tests to speed them up by skipping the Sync operations. This slowdown is especially noticeable on the Mac with the FULL_SYNC.