[Draft] Hotfix: Add env variables to overcome A3 highgpu failure#5291
Conversation
Summary of ChangesHello @SwarnaBharathiMantena, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical issue causing NCCL test failures on A3 HighGPU nodes by ensuring that Slurm's external prolog and epilog wrapper scripts correctly set the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses a critical issue with NCCL test failures on A4 HighGPU nodes by ensuring the SLURM_SCRIPT_CONTEXT environment variable is set correctly in the prolog and epilog wrapper scripts. The fix is direct and well-explained in the description. My review includes a couple of suggestions to add comments to the code, which will improve long-term maintainability by explaining the purpose of this variable directly in the scripts, in line with the project's style guide. The provided rule regarding variable configurability does not apply to these comments, as they focus on documenting hardcoded values rather than making them configurable.
d574070 to
8a32650
Compare
|
Did not fix the issue. Debugging futher. |
The NCCL test failure with WEXITSTATUS 3 was caused by a missing environment variable in the Slurm Prolog/Epilog wrapper scripts.
Root Cause Analysis
Sockets in /run/tcpx-${SLURM_JOB_ID}. These sockets are then mounted into the Enroot container.
SLURM_SCRIPT_CONTEXT environment variable to determine which scripts to run (e.g., prolog_slurmd or epilog_slurmd).
SLURM_SCRIPT_CONTEXT. Consequently, slurm_mux skipped all scripts, including rxdm.
nodes. When srun attempted to start the container, the mount operation failed because the source directory did not exist,
leading to exit code 3.
Fixes Applied
Updated the following files in the schedmd-slurm-gcp-v6-controller module:
community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/files/external_prolog.sh: Now exportsSLURM_SCRIPT_CONTEXT="prolog_slurmd".
community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/files/external_epilog.sh: Now exportsSLURM_SCRIPT_CONTEXT="epilog_slurmd".
These changes ensure that custom prologs and epilogs (including TCPX setup) are correctly identified and executed by the multiplexer.