docs(proposals): enhance parallel processing proposal#419
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vibhor-5 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @vibhor-5! It looks like this is your first PR to kubeedge/ianvs 🎉 |
There was a problem hiding this comment.
Code Review
This pull request introduces a proposal for parallel test case execution in the Ianvs benchmarking framework using Python's ProcessPoolExecutor. The design emphasizes backward compatibility and configurable parallelism. Review feedback highlights the need to handle single-core systems in the default worker count calculation, maintain consistency between architectural diagrams and workflow descriptions regarding the 'Rank' component, and ensure the impact analysis table comprehensively lists all modified files like init.py.
|
|
||
| # Risk Assessment & DoD | ||
|
|
||
| - **Risk**: Resource Exhaustion (OOM). **Mitigation**: Conservative `cpu_count-1` default and Phase 1.5 research. |
There was a problem hiding this comment.
The proposal suggests a default worker count of cpu_count-1. On single-core systems, this results in 0 workers, which causes ProcessPoolExecutor to fail. Using max(1, cpu_count() - 1) ensures at least one worker is always used.
| - **Risk**: Resource Exhaustion (OOM). **Mitigation**: Conservative `cpu_count-1` default and Phase 1.5 research. | |
| - **Risk**: Resource Exhaustion (OOM). **Mitigation**: Conservative max(1, cpu_count() - 1) default and Phase 1.5 research. |
|
|
||
| ## End-to-End Workflow | ||
|
|
||
| > CLI parses `--parallel` args $\to$ `BenchmarkingJob` sets internal fields $\to$ `TestCaseController` builds test cases $\to$ `run_testcases()` enters enhanced parallel branch $\to$ `ProcessPoolExecutor` manages workers $\to$ `Rank` saves results. |
| | `benchmarking.py` | CLI argument parsing | Added `-p` and `-w` as optional. Default maintains current behavior. | | ||
| | `benchmarkingjob.py` | Configuration parsing | CLI overrides applied *after* YAML; standard priority. | | ||
| | `testcasecontroller.py` | `run_testcases` method | Side-by-side implementation. Serial branch is a direct copy of production. | | ||
| | `testcase.py` | Worker function | New top-level function. Existing `TestCase.run` remains unchanged. | |
There was a problem hiding this comment.
Line 135 mentions __init__.py is modified for the worker function, but it is missing from this impact analysis table. Consider including it for completeness.
| | `testcase.py` | Worker function | New top-level function. Existing `TestCase.run` remains unchanged. | | |
| | testcase.py and __init__.py | Worker function | New top-level function in testcase.py, exported via __init__.py. Existing TestCase.run remains unchanged. | |
… reviewer feedback - Updated Lay-0 architecture with L1/L2/L3 layered structure. - Enhanced AI paradigm analysis with deeper Ianvs integration details. - Added default worker count research plan and future dynamic settings. - Strengthened backward compatibility justifications and validation plan. - Consolidated all parallel-related research into a single proposal file. Addresses reviewer comments for PR kubeedge#308. Signed-off-by: Krrish Biswas <krrish175-byte@users.noreply.github.com> Signed-off-by: vibhor kumar <vibhork1105@gmail.com>
Overview
This PR updates the parallel test case processing proposal (#8) and follows on from PR #308. It consolidates research, refines the architectural design, and strengthens backward compatibility guarantees.
Key Changes
Consolidation
All parallel-related research findings and design justifications are now consolidated into a single proposal file: docs/proposals/chore/parallel-processing/parallel-testcase-processing-proposal.md.
Checklist
Note: This PR focuses strictly on the proposal documentation. Implementation will follow once the refined design is approved.