feat(npi): configure 48-thread default for RAPID buckets and add explicit numjobs override - #191
Conversation
…icit numjobs override - Set default NUMJOBS=48 concurrency for Zonal RAPID buckets across GCE and GKE benchmark runners while preserving NUMJOBS=112 default for standard regional buckets. - Add --numjobs CLI argument in npi.py and npi_gke.py to allow custom job concurrency overrides. - Update FIO benchmark runner to dynamically resolve num_jobs from environment variables (NUM_JOBS, NUMJOBS) when pre-creating benchmark directories. - Add unit tests verifying NUMJOBS environment variable propagation and overrides for GCE and GKE benchmark spec generation.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for configuring and overriding the 'numjobs' concurrency count in benchmark runs across both local Docker and GKE environments, including a default of 48 for RAPID buckets. It also adds corresponding CLI arguments and unit tests. The feedback suggests simplifying the redundant argument-parsing logic in GKE job spec creation to improve readability.
| if "--numjobs=2" in args: | ||
| num_jobs_val = "2" | ||
| elif any(arg.startswith("--numjobs=") for arg in args): | ||
| num_jobs_val = "112" | ||
| for arg in args: | ||
| if arg.startswith("--numjobs="): | ||
| num_jobs_val = arg.split("=", 1)[1] | ||
| break | ||
| elif is_rapid_bucket: | ||
| num_jobs_val = "48" | ||
| else: | ||
| num_jobs_val = "112" |
There was a problem hiding this comment.
The logic for resolving num_jobs_val from args contains redundant checks. Specifically, checking "--numjobs=2" in args is redundant because the subsequent startswith("--numjobs=") loop will already correctly match and extract "2". Additionally, checking any(arg.startswith("--numjobs=") for arg in args) before looping is also redundant as the loop itself can handle the search. We can simplify this block to improve readability and maintainability.
num_jobs_val = None
for arg in args:
if arg.startswith("--numjobs="):
num_jobs_val = arg.split("=", 1)[1]
break
if num_jobs_val is None:
num_jobs_val = "48" if is_rapid_bucket else "112"1 participant
Summary
NUMJOBS=48concurrency for Zonal RAPID buckets across GCE (npi.py) and GKE (npi_gke.py) benchmark runners, while preservingNUMJOBS=112default for standard Regional buckets.--numjobsCLI argument innpi.pyandnpi_gke.pyto allow custom job concurrency overrides.fio_benchmark_runner.py) to dynamically resolvenum_jobsfrom environment variables (NUM_JOBS,NUMJOBS) when pre-creating benchmark directories.npi_test.pyandnpi_gke_test.pycovering standard, RAPID, smoke mode, and explicit--numjobsoverrides.Testing
python3 -m unittest discover -p "*_test.py") — all 123 tests passed.