-
Notifications
You must be signed in to change notification settings - Fork 4
feat: allow configuring clear-cache statically and injecting torn writes at runtime #7
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
Conversation
| persist=[1,3] | ||
|
|
||
| [[injection]] | ||
| type="clear" |
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.
A couple of suggestions here before diving deep into a review:
-
If we want to add a
type="clear"fault, then we must maintain the already known syntax, which istype=clear-cache, which indicates that specifyingcrash=false(assume default) has the same effect has sendinglazyfs::clear-cacheto the faults fifo.- With this new type, and the option to specify
crash=true, we must removelazyfs::crashfrom the API and replace bylazyfs::clear-cachewith more options, namely thetiming,op,occurrence, andcrash(e.g.,lazyfs::clear-cache::timing=...::op=...::occurrence=...::crash), wherecrashmay be omitted (optional parameter). - We must also ensure the user never pre-configures a
clear-cachewithout atimingandop, but he can sendlazyfs::clear-cache::crashto the fifo to simulate "crash right now".
- With this new type, and the option to specify
-
Instead of the above, why can't we just add support for the existing
lazyfs::crashfault under the config file? Do we need to specify a timing for the clear cache without crashing LazyFS?
WDYT?
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.
TL;DR: We going with option 1, but maintaining the crash fault option for now and will probably deprecate it soon. @mj-ramos Can you please change the fault syntax of the clear fault type to clear-cache?
…nnot be added in runtime
devzizu
left a comment
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.
We'll leave some of the suggestions for later improvements.
API update: