-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: allow configuring clear-cache statically and injecting torn writes at runtime #7
Conversation
@@ -93,6 +93,14 @@ file="output1.txt" | |||
occurrence=5 | |||
parts=3 #or parts_bytes=[4096,3600,1260] | |||
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-cache
to the faults fifo.- With this new type, and the option to specify
crash=true
, we must removelazyfs::crash
from the API and replace bylazyfs::clear-cache
with more options, namely thetiming
,op
,occurrence
, andcrash
(e.g.,lazyfs::clear-cache::timing=...::op=...::occurrence=...::crash
), wherecrash
may be omitted (optional parameter). - We must also ensure the user never pre-configures a
clear-cache
without atiming
andop
, but he can sendlazyfs::clear-cache::crash
to 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::crash
fault 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
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: