Add initial version of benchmark experiment runner #1266
Conversation
Signed-off-by: Daniel Carl Jones <[email protected]>
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
benchmark/benchmark.py
Outdated
| try: | ||
| # TODO: Add resource monitoring during FIO job | ||
| _run_fio(cfg, mount_dir) | ||
| success = True |
There was a problem hiding this comment.
try/except/else also is probably wanted here as well
There was a problem hiding this comment.
I don't follow what we want the else for?
There was a problem hiding this comment.
Should we add a section on benchmarks and how to create a new one?
There was a problem hiding this comment.
Added a short desc on how the benchmark works, where its defined
benchmark/benchmark.py
Outdated
| mp_version = _mount_mp(cfg, metadata, mount_dir) | ||
| mounted = True | ||
| metadata["mp_version"] = mp_version |
There was a problem hiding this comment.
Would it be useful to parameterise mountpoint flags? If so, we could also add them to metadata.
There was a problem hiding this comment.
I'm not sure what you mean here, can you rephrase?
benchmark/README.md
Outdated
| This will run the default experiment, including many different configuration combinations. | ||
| Output is written to `multirun/` within directories for the date, time, and job run. |
There was a problem hiding this comment.
Maybe it could be nice to include an example output?
There was a problem hiding this comment.
I described the output a bit more. I'm worried it'll be hard to maintain and become outdated quickly here.
Signed-off-by: Daniel Carl Jones <[email protected]>
Signed-off-by: Daniel Carl Jones <[email protected]>
Signed-off-by: Daniel Carl Jones <[email protected]>
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
| ] | ||
| subprocess_env = {} | ||
|
|
||
| if cfg['s3_prefix'] is not None: |
There was a problem hiding this comment.
Should we just use mountpoint args as they are than have a different mapping? For example, we could just use prefix instead of s3_prefix, debug-crt instead of mountpoint_debug_crt etc.,
There was a problem hiding this comment.
My thinking is that it doesn't map very well as a straight forward mapping. Right now, it wraps both Mountpoint and FIO - maybe we might extend either (more benchmarking tools or more layers of MP, though I'm not confident on how well it can be generalized).
Additionally, there's the parameters mentioned in this script as well as when they are read in Pandas when analysing results. I diverged from MP naming to try and make the results clearer (for example, using fuse_threads instead of max-threads).
I'm leaning more to keep them diverged for now.
Signed-off-by: Daniel Carl Jones <[email protected]>
dannycjones
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
dannycjones
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
dannycjones
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
dannycjones
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
dannycjones
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
dannycjones
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
dannycjones
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
…n.dump Signed-off-by: Daniel Carl Jones <[email protected]>
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
dannycjones
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
|
LGTM |
In order to investigate performance in Mountpoint, we want to be able to vary different parameters. In fact, it can be very useful to vary these parameters together to see how performance (such as sequential read throughput) changes as we vary two parameters together.
This change introduces a new benchmark running script which uses the Python framework Hydra to enumerate combinations of parameters, and then execute some function with each combination. The script manages the lifecycle of the
mount-s3file system and collecting data into an output folder.The change currently does not reuse the FIO definitions used by our regression benchmarks. In the mid-term, these should be reconciled.
This pull request (PR) supersedes a previous PR: #986.
Does this change impact existing behavior?
No, this adds a new benchmark runner and benchmark definitions. This does not impact the Mountpoint file system.
Does this change need a changelog entry? Does it require a version change?
No, no impact to Mountpoint file system or crates.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).