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

vine: reserve a factor of disk when allocating resources #4035

Conversation

JinZhou5042
Copy link
Member

@JinZhou5042 JinZhou5042 commented Jan 24, 2025

Proposed Changes

Part of #4006

Testing results using 32 4-core workers

  • Before
    image

  • After
    image

Merge Checklist

The following items must be completed before PRs can be merged.
Check these off to verify you have completed all steps.

  • make test Run local tests prior to pushing.
  • make format Format source code to comply with lint policies. Note that some lint errors can only be resolved manually (e.g., Python)
  • make lint Run lint on source code prior to pushing.
  • Manual Update: Update the manual to reflect user-visible changes.
  • Type Labels: Select a github label for the type: bugfix, enhancement, etc.
  • Product Labels: Select a github label for the product: TaskVine, Makeflow, etc.
  • PR RTM: Mark your PR as ready to merge.

@JinZhou5042 JinZhou5042 self-assigned this Jan 24, 2025
@JinZhou5042 JinZhou5042 changed the title vine: vine: reserve a factor of disk when allocating resources Jan 24, 2025
@JinZhou5042 JinZhou5042 requested review from btovar and dthain January 24, 2025 21:43
@JinZhou5042
Copy link
Member Author

JinZhou5042 commented Jan 28, 2025

We are alomost there. @btovar, could you confirm if factoring the disk just before rmsummary_merge_max(limits, min); is the right way to do it?

Are the position and the condition on proportional resources correct?

......

if (q->proportional_resources) {
	limits->disk *= q->disk_proportion_available_to_task;
}

rmsummary_merge_max(limits, min);
rmsummary_merge_override_basic(limits, t->resources_requested);

return limits;

@btovar
Copy link
Member

btovar commented Jan 29, 2025

I think you want limits->disk *= q->disk_proportion_available_to_task; always, not only with proportional resources? The reasoning is that it is a policy for the cache?

@JinZhou5042 JinZhou5042 requested review from btovar and dthain January 29, 2025 17:54
@btovar btovar merged commit 04724ee into cooperative-computing-lab:master Jan 30, 2025
10 checks passed
@JinZhou5042 JinZhou5042 deleted the resource_allocation_for_pythontasks branch January 30, 2025 14:21
btovar added a commit that referenced this pull request Feb 6, 2025
* init

* tune param

* lint

* lint

* lint

* condition cahnge: value > 1 || value <= 0

* reserve disk not only for proportional

* lint

* only modify disk factor when in range (0,1)

---------

Co-authored-by: Benjamin Tovar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants