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

Maybe job's min resource need to considering minMember or min avainable number of each role ? #2791

Open
lowang-bh opened this issue Apr 15, 2023 · 2 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@lowang-bh
Copy link
Member

What would you like to be added:

func (ji *JobInfo) GetMinResources() *Resource {
	if ji.PodGroup.Spec.MinResources == nil {
                // a function to sum up minMember's tasks' reqeust resource
		return ji.CalMinResources() 
	}

	return NewResource(*ji.PodGroup.Spec.MinResources)
}

Why is this needed:

As elastic resource define in queueAttr, it is some of job's elastic resource, which equals to job.allocated - job.minAvailable.
But when cal the min resource for job, it return empty when ji.PodGroup.Spec.MinResources == nil . This is not corresponding with code in controller:

MinResources: cc.calcPGMinResources(job),

type queueAttr struct {
    // elastic represents the sum of job's elastic resource, job's elastic = job.allocated - job.minAvailable
    elastic *api.Resource
}

// 2. elastic sums up queue's jobs' elastic resource
attr.elastic.Add(job.GetElasticResources())

// 3.  Here, when MinResources is nil,  shall we calculate it by minMember according to task priority?
// so that it can be corresponde with function at pkg/controllers/job/job_controller_actions.go#L686
// GetMinResources return the min resources of podgroup.
func (ji *JobInfo) GetMinResources() *Resource {
	if ji.PodGroup.Spec.MinResources == nil {
		return EmptyResource()
	}

	return NewResource(*ji.PodGroup.Spec.MinResources)
}

func (ji *JobInfo) GetElasticResources() *Resource {
	if ji.Allocated.LessEqualPartly(ji.GetMinResources(), Zero) {
		return EmptyResource()
	}
	return ji.Allocated.Clone().Sub(ji.GetMinResources())
}
@lowang-bh lowang-bh added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 15, 2023
@lowang-bh lowang-bh changed the title May be job's min resource need to considering minMember or min avainable number of each role ? Maybe job's min resource need to considering minMember or min avainable number of each role ? Apr 15, 2023
@lowang-bh
Copy link
Member Author

What's more, let admission webhook or controller to calculate the minResource for normal podgroups when it is submit, like what's done in vcjob controller

@lowang-bh
Copy link
Member Author

lowang-bh commented Aug 15, 2023

/assign

refer to PR fix calculations of podgroup min resource #3057 to get podgroup min resource

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

1 participant