Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Untangle exclusive and enable_placement, update examples. #3362

Merged
merged 2 commits into from
Jan 19, 2025

Conversation

mr0re1
Copy link
Collaborator

@mr0re1 mr0re1 commented Dec 7, 2024

Motivations:

  • Enable usage of placements with dense reservations.

  • Encourage usage of placements by making them to always work regardless of nodeset configuration.

Done:
Remove constraints:

  • "If a reservation is specified, var.enable_placement must be false" - dense reservations support external placements, non-dense reservation will ignore Slurm-created placement.

  • "If any non-static nodesets has enable_placement, var.exclusive must be set true" - nodeset-wise (non-exclusive) placement can be used with dynamic nodes.

Remove enable_placement: false from SlurmV6 examples.

Reasons for users to set enable_placement=false:

  • If non-dense reservation is used, user can avoid extra-cost of creating placement policies;
  • If customer wants to avoid "all or nothing" VM provisioning behaviour;
  • If customer wants to intentionally have "spread" VMs (e.g. for reliability reasons)

@mr0re1 mr0re1 added the release-chore To not include into release notes label Dec 7, 2024
@mr0re1 mr0re1 requested a review from alyssa-sm December 7, 2024 17:29
@mr0re1 mr0re1 changed the title Relax constraint around reservation and static nodesets with placement Untangle exclusive and enable_placement, update examples. Dec 7, 2024
@mr0re1 mr0re1 requested review from tpdownes and cboneti December 7, 2024 20:41
@alyssa-sm
Copy link
Contributor

There is a error in PR-validation with one of the validate_golden_copy failing that should be addressed but otherwise LGTM

@alyssa-sm alyssa-sm assigned mr0re1 and unassigned alyssa-sm Dec 10, 2024
@mr0re1 mr0re1 force-pushed the res_pp branch 2 times, most recently from ebde2b9 to 955dc44 Compare January 10, 2025 21:44
@mr0re1 mr0re1 assigned cboneti and unassigned mr0re1 Jan 10, 2025
@mr0re1 mr0re1 added the do-not-merge Block merging of this PR label Jan 10, 2025
@mr0re1 mr0re1 added release-improvements Added to release notes under the "Improvements" heading. and removed release-chore To not include into release notes do-not-merge Block merging of this PR labels Jan 19, 2025
@mr0re1 mr0re1 merged commit 8b711f6 into GoogleCloudPlatform:develop Jan 19, 2025
11 of 57 checks passed
@mr0re1 mr0re1 deleted the res_pp branch January 19, 2025 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-improvements Added to release notes under the "Improvements" heading.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants