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

Added job's MinResource check once job is poped from queue #3055

Closed
wants to merge 4 commits into from
Closed

Added job's MinResource check once job is poped from queue #3055

wants to merge 4 commits into from

Conversation

srikanth-iyengar
Copy link

@srikanth-iyengar srikanth-iyengar commented Aug 14, 2023

fix #3047

This will save time

Signed-off-by: Iyengar, Srikanth <[email protected]>
@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign thor-wl
You can assign the PR to them by writing /assign @thor-wl in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@volcano-sh-bot volcano-sh-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 14, 2023
@srikanth-iyengar
Copy link
Author

/hold

@volcano-sh-bot volcano-sh-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 14, 2023
@srikanth-iyengar
Copy link
Author

Hey @lowang-bh can you review this PR ?

@srikanth-iyengar
Copy link
Author

hey @lowang-bh can you review this pr ?

@lowang-bh
Copy link
Member

hey @lowang-bh can you review this pr ?

I think you can first get job's minResource, and use compare function in the lib.

@srikanth-iyengar
Copy link
Author

/assign

@volcano-sh-bot volcano-sh-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 25, 2023
@srikanth-iyengar
Copy link
Author

/test all

@volcano-sh-bot
Copy link
Contributor

@srikanth-iyengar: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test all

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@srikanth-iyengar
Copy link
Author

srikanth-iyengar commented Aug 25, 2023

@lowang-bh will this work?

                jobMinResource := job.GetMinResources()
		queueAllocatedResource := api.NewResource(queue.Queue.Status.Allocated)
		queueCapability := api.NewResource(queue.Queue.Spec.Capability)
		if jobMinResource.Add(queueAllocatedResource).Less(queueCapability, api.Zero) {
			klog.V(4).Infof("Queue <%s> is overused when considering job <%s>, ignore it.", queue.Name, job.Name)
			continue
		}

@srikanth-iyengar
Copy link
Author

Hey @lowang-bh Can you check on the changes ?

@lowang-bh
Copy link
Member

Hey @lowang-bh Can you check on the changes ?

How about first to pick some good first issue, eg the UTs, to familar with volcano? It will help you to understand the full archtechure and scheduling process.

@srikanth-iyengar srikanth-iyengar removed their assignment Aug 28, 2023
@srikanth-iyengar
Copy link
Author

Yeah, Got it thanks for suggestions, I guess it will really help me

Copy link

stale bot commented Dec 15, 2023

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 15, 2023
@srikanth-iyengar srikanth-iyengar closed this by deleting the head repository Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[good first issue]It's better to check and skip allocation for job when it is poped
3 participants