[Robustness] Add additional fio workloads and fix fio runner #529
Conversation
Add more fio workloads to write files at different depths in random branches of the generated file system tree. - Write files at depth - Write files at a specified depth, creating a new directory branch at a random depth - Delete a random directory at a given depth - Delete some or all of the contents of a random directory at a specified depth
|
AppVeyor failure does not look related to this change: @jkowalski is there a way to retrigger? Either I don't have permission or I can't find the button that does it :) |
| } | ||
| } | ||
|
|
||
| rand.Shuffle(len(dirList), func(i, j int) { |
There was a problem hiding this comment.
Since the operation ends up being exhaustive at a given depth, what's the objective or benefit of shuffling the order?
There was a problem hiding this comment.
I think I understand what your concern is, correct me if I'm wrong.
Yes we'll do an exhaustive search in a given branch, if only a certain subset of directories has the requested depth to conduct the operation.
But if I call, e.g., DeleteDirAtDepth("/", 4) 2 times in a row, I don't want to be limited to only deleting from the initial alphabetical subdirectory /aaaa/aaaa/aaaa/delete_this_directory_first, then /aaaa/aaaa/aaaa/delete_this_directory_second. I actually want a chance of deleting /zzzz/zzzz/zzzz/delete_this_too_by_chance. Since it's recursive, I'll shuffle the subdirectory traversal pick at any given depth so I give a chance to other viable paths that can complete the operation.
There was a problem hiding this comment.
I'm confused.
Is operateAtDepth expected to execute f on a single directory a depth depth? or on all directories that happen to be found at the specified depth?
Should the loop below break, actually return, when err == nil ?
Or is the comment above regarding the case when an error is encountered and then the operation is repeated?
There was a problem hiding this comment.
operateAtDepth is "keep traversing into deeper subdirectories, picking a random subdirectory each time. Then when you pick a subdirectory that is at the requested depth, pass the path to that subdirectory into the function f to do whatever's needed. For example Deleting a directory at that depth is just f = os.RemoveAll. Deleting the contents of a directory at that depth but leaving the directory itself involves reading all of the contents and calling os.RemoveAll.
We only want to choose among paths that are as deep as the requested depth, so iterating through the subdirectories is only performed when a given branch's depth is not sufficient, which is what I was referring to in the comment above.
/ab
/aa/aa/aa/delete_this_directory
If it's looking for depth 4, it might pick /ab first, then when it doesn't find any subdirectories to further traverse, pops with ErrNoDirFound, in which case the parent loop will iterate within that parent directory and try /aa. That path will end up with a successful operation at depth 4.
There was a problem hiding this comment.
OK, but what are the expected semantics?
In the example above with 3 directories at the same level:
/aaaa/aaaa/aaaa/delete_this_directory_first/aaaa/aaaa/aaaa/delete_this_directory_second/zzzz/zzzz/zzzz/delete_this_too_by_chance
If DeleteDirAtDepth("/", 4) is called, does a single (random) directory get removed? or do all of them get removed?
There was a problem hiding this comment.
The expectation is a single random directory. As soon as a successful operation takes place (e.g. os.RemoveAll), the operateAtDepth stack unwinds, returning err == nil (or any other error, as long as it's not ErrNoDirFound, which only happens if it couldn't find a directory to execute the operation on)
| } | ||
|
|
||
| // DeleteContentsAtDepth deletes some or all of the contents of a directory | ||
| // at the provided depths. |
There was a problem hiding this comment.
It may be good to describe what pcnt is, and express it as a probability instead of a percentage.
|
Thanks for the added scrutiny @julio-lopez! Great to re-evaluate this code. |
Followup on recent PR #529, some suggestions and discussion after it was merged: - Express probability as float in range [0,1] - Add a unit test for DeleteContentsAtDepth - Add a comment on writeFilesAtDepth explaining depth vs branchDepth - Refactor pickRandSubdirPath for easier readability and understanding Upon some reflection, I decided to refactor pickRandSubdirPath() to gather indexes and pick randomly from them instead of the previous reservoir sampling approach. I think this is easier to understand going forward without extra explanation, doesn't have much additional memory overhead, and reduces the number of rand calls to 1.
Followup on recent PR #529, some suggestions and discussion after it was merged: - Express probability as float in range [0,1] - Add a unit test for DeleteContentsAtDepth - Add a comment on writeFilesAtDepth explaining depth vs branchDepth - Refactor pickRandSubdirPath for easier readability and understanding Upon some reflection, I decided to refactor pickRandSubdirPath() to gather indexes and pick randomly from them instead of the previous reservoir sampling approach. I think this is easier to understand going forward without extra explanation, doesn't have much additional memory overhead, and reduces the number of rand calls to 1.
Add more fio workloads to write files at different depths in random
branches of the generated file system tree.
a random depth
a specified depth
Additionally, apply fixes to fio runner similar to what was done in #293 for kopia e2e test runner. Add debug flag to suppress logs unless Debug is set