feat(scheduler): support resource quota consideration during pod group enqueue procedure

[merryzhou]

#1014

[zen-xu]

Add event will be more user-friendly

[Thor-wl]

/assign @william-wang @wpeng102 @shinytang6 @zen-xu Please take a review.

[merryzhou]

Add event will be more user-friendly

done

[shinytang6]

l think we also need a doc for this plugin ?

[Robert-Christensen-visa]

This plugin would be useful and would resolve an issue we are encountering in our setup, so I hope it gets merged soon. This is really great!

It looks like there might be a corner case this is not covered covered.

I assume this plugin will look at pending podgroups and will enqueue them only if there is enough capacity in the namespace according to Kubernetes ResourceQuota.

If many jobs small jobs are submitted to a namespace at the same time, it is possible for all the jobs to be enqueued because this plugin is not keeping state of previous podgroups that have been permitted in the current scheduling round. Take this example:

apiVersion: v1
kind: ResourceQuota
metadata:
  name: small-quota
spec:
  hard:
    cpu: "2"

Say I have many small jobs I want to queue in the namespace small that has the above ResourceQuota. Here is a template I used. I saved this file as job_template.json:

apiVersion: batch.volcano.sh/v1alpha1
kind: Job
metadata:
  name: vcjob-job-<id>
spec:
  minAvailable: 1
  schedulerName: volcano
  tasks:
    - replicas: 1
       name: "worker"
       template:
        spec:
          containers:
            - image: ubuntu
              imagePullPolicy: IfNotPresent
              name: ubuntu
              command: ["sh", "-c", "sleep 60"]
              resources:
                limits:
                 cpu: "1"
                 memory: 64Mi
                requests:
                 cpu: "1"
                 memory: 64Mi
            restartPolicy: OnFailure

I submit many of these jobs at once, using this script:

for id in $(seq 32)
do
  sed "s/<id>/job-$id/g" < job_template.json | \
  kubectl apply -n small -f -
done

When each job is considered individually using the resourcequota plugin, they will all be permitted. I don't think this should be the case, I think the plugin will need to consider other podgroups that have already been permitted in the scheduling round to prevent it from enqueueing too many podgroups and exceeding the namespace resource quota.

I have made some suggestions that would fix this issue. This assumes the order of jobs submitted is in priority order.

[Robert-Christensen-visa]

Suggested change

	func (rq *resourceQuotaPlugin) OnSessionOpen(ssn *framework.Session) {
	ssn.AddJobEnqueueableFn(rq.Name(), func(obj interface{}) int {
	job := obj.(*api.JobInfo)
	func (rq *resourceQuotaPlugin) OnSessionOpen(ssn *framework.Session) {
	pendingResources := make(map[string]v1.ResourceList)
	ssn.AddJobEnqueueableFn(rq.Name(), func(obj interface{}) int {
	job := obj.(*api.JobInfo)

[merryzhou]

done

[Robert-Christensen-visa]

Suggested change

	hardResources := quota.ResourceNames(resourceQuota.Hard)
	requestedUsage := quota.Mask(*resourcesRequests, hardResources)
	newUsage := quota.Add(resourceQuota.Used, requestedUsage)
	maskedNewUsage := quota.Mask(newUsage, quota.ResourceNames(requestedUsage))
	hardResources := quota.ResourceNames(resourceQuota.Hard)
	requestedUsage := quota.Mask(*resourcesRequests, hardResources)
	var resourcesUsed = resourceQuota.Used
	if pendingUse, found := pendingResources[job.Namespace]; found {
	resourcesUsed = quota.Add(pendingUse, resourcesUsed)
	}
	newUsage := quota.Add(resourcesUsed, requestedUsage)
	maskedNewUsage := quota.Mask(newUsage, quota.ResourceNames(requestedUsage))

[Robert-Christensen-visa]

Suggested change

	}
	return util.Permit
	}
	if _, found := pendingResources[job.Namespace]; !found {
	pendingResources[job.Namespace] = v1.ResourceList{}
	}
	pendingResources[job.Namespace] = quota.Add(pendingResource[job.Namespace], *resourcesRequests)
	return util.Permit

[Thor-wl]

This plugin would be useful and would resolve an issue we are encountering in our setup, so I hope it gets merged soon. This is really great!

It looks like there might be a corner case this is not covered covered.

I assume this plugin will look at pending podgroups and will enqueue them only if there is enough capacity in the namespace according to Kubernetes ResourceQuota.

If many jobs small jobs are submitted to a namespace at the same time, it is possible for all the jobs to be enqueued because this plugin is not keeping state of previous podgroups that have been permitted in the current scheduling round. Take this example:

apiVersion: v1
kind: ResourceQuota
metadata:
  name: small-quota
spec:
  hard:
    cpu: "2"

Say I have many small jobs I want to queue in the namespace small that has the above ResourceQuota. Here is a template I used. I saved this file as job_template.json:

apiVersion: batch.volcano.sh/v1alpha1
kind: Job
metadata:
  name: vcjob-job-<id>
spec:
  minAvailable: 1
  schedulerName: volcano
  tasks:
    - replicas: 1
       name: "worker"
       template:
        spec:
          containers:
            - image: ubuntu
              imagePullPolicy: IfNotPresent
              name: ubuntu
              command: ["sh", "-c", "sleep 60"]
              resources:
                limits:
                 cpu: "1"
                 memory: 64Mi
                requests:
                 cpu: "1"
                 memory: 64Mi
            restartPolicy: OnFailure

I submit many of these jobs at once, using this script:

for id in $(seq 32)
do
  sed "s/<id>/job-$id/g" < job_template.json | \
  kubectl apply -n small -f -
done

When each job is considered individually using the resourcequota plugin, they will all be permitted. I don't think this should be the case, I think the plugin will need to consider other podgroups that have already been permitted in the scheduling round to prevent it from enqueueing too many podgroups and exceeding the namespace resource quota.

I have made some suggestions that would fix this issue. This assumes the order of jobs submitted is in priority order.

Thanks for your advice. The request resources of podgroups, which has already been admitted to be enqueued at current scheduling round, should be considered as used in namespace quota. This feature is important and we will compelete review as soon as possible when the developer resolved the comments and conflicts.

[merryzhou]

l think we also need a doc for this plugin ?

#1583

[Thor-wl]

Is this PR still needed? @merryzhou

[stale]

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.

[k82cn]

Let's rename RQStatus to QuotaStatus :)

[Thor-wl]

+1
I agree with that QuotaStatus may be more readable.

[merryzhou]

done

[william-wang]

@merryzhou Overall, looks good to me. Please make the ci happy.

[merryzhou]

@merryzhou Overall, looks good to me. Please make the ci happy.

done

[Thor-wl]

/lgtm

[shinytang6]

LGTM, l think it's ready to go!
/cc @Thor-wl @william-wang

[Thor-wl]

/approve

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Thor-wl

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Thor-wl]

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