From 9341722654db8d3d96ff178be36e3fe8628e68c7 Mon Sep 17 00:00:00 2001
From: Ziyang Wu <zionwu3@gmail.com>
Date: Thu, 23 May 2019 10:02:18 +0800
Subject: [PATCH] Fix wrong calculation for queue deserved in proportion plugin

---
 pkg/scheduler/api/resource_info.go            | 35 +++++++++++++++++++
 .../plugins/proportion/proportion.go          | 18 +++++++---
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/pkg/scheduler/api/resource_info.go b/pkg/scheduler/api/resource_info.go
index 956669469d..b575e49174 100644
--- a/pkg/scheduler/api/resource_info.go
+++ b/pkg/scheduler/api/resource_info.go
@@ -277,6 +277,41 @@ func (r *Resource) LessEqual(rr *Resource) bool {
 	return true
 }
 
+// Diff calculate the difference between two resource
+func (r *Resource) Diff(rr *Resource) (*Resource, *Resource) {
+	increasedVal := EmptyResource()
+	decreasedVal := EmptyResource()
+	if r.MilliCPU > rr.MilliCPU {
+		increasedVal.MilliCPU += r.MilliCPU - rr.MilliCPU
+	} else {
+		decreasedVal.MilliCPU += rr.MilliCPU - r.MilliCPU
+	}
+
+	if r.Memory > rr.Memory {
+		increasedVal.Memory += r.Memory - rr.Memory
+	} else {
+		decreasedVal.Memory += rr.Memory - r.Memory
+	}
+
+	for rName, rQuant := range r.ScalarResources {
+		rrQuant := rr.ScalarResources[rName]
+
+		if rQuant > rrQuant {
+			if increasedVal.ScalarResources == nil {
+				increasedVal.ScalarResources = map[v1.ResourceName]float64{}
+			}
+			increasedVal.ScalarResources[rName] += rQuant - rrQuant
+		} else {
+			if decreasedVal.ScalarResources == nil {
+				decreasedVal.ScalarResources = map[v1.ResourceName]float64{}
+			}
+			decreasedVal.ScalarResources[rName] += rrQuant - rQuant
+		}
+	}
+
+	return increasedVal, decreasedVal
+}
+
 // String returns resource details in string format
 func (r *Resource) String() string {
 	str := fmt.Sprintf("cpu %0.2f, memory %0.2f", r.MilliCPU, r.Memory)
diff --git a/pkg/scheduler/plugins/proportion/proportion.go b/pkg/scheduler/plugins/proportion/proportion.go
index 75bb1537ed..10f951cf24 100644
--- a/pkg/scheduler/plugins/proportion/proportion.go
+++ b/pkg/scheduler/plugins/proportion/proportion.go
@@ -111,11 +111,15 @@ func (pp *proportionPlugin) OnSessionOpen(ssn *framework.Session) {
 
 		// If no queues, break
 		if totalWeight == 0 {
+			glog.V(4).Infof("Exiting when total weight is 0")
 			break
 		}
 
 		// Calculates the deserved of each Queue.
-		deserved := api.EmptyResource()
+		// increasedDeserved is the increased value for attr.deserved of processed queues
+		// decreasedDeserved is the decreased value for attr.deserved of processed queues
+		increasedDeserved := api.EmptyResource()
+		decreasedDeserved := api.EmptyResource()
 		for _, attr := range pp.queueOpts {
 			glog.V(4).Infof("Considering Queue <%s>: weight <%d>, total weight <%d>.",
 				attr.name, attr.weight, totalWeight)
@@ -125,20 +129,26 @@ func (pp *proportionPlugin) OnSessionOpen(ssn *framework.Session) {
 
 			oldDeserved := attr.deserved.Clone()
 			attr.deserved.Add(remaining.Clone().Multi(float64(attr.weight) / float64(totalWeight)))
-			if !attr.deserved.LessEqual(attr.request) {
+
+			if attr.request.Less(attr.deserved) {
 				attr.deserved = helpers.Min(attr.deserved, attr.request)
 				meet[attr.queueID] = struct{}{}
+				glog.V(4).Infof("queue <%s> is meet", attr.name)
+
 			}
 			pp.updateShare(attr)
 
 			glog.V(4).Infof("The attributes of queue <%s> in proportion: deserved <%v>, allocate <%v>, request <%v>, share <%0.2f>",
 				attr.name, attr.deserved, attr.allocated, attr.request, attr.share)
 
-			deserved.Add(attr.deserved.Clone().Sub(oldDeserved))
+			increased, decreased := attr.deserved.Diff(oldDeserved)
+			increasedDeserved.Add(increased)
+			decreasedDeserved.Add(decreased)
 		}
 
-		remaining.Sub(deserved)
+		remaining.Sub(increasedDeserved).Add(decreasedDeserved)
 		if remaining.IsEmpty() {
+			glog.V(4).Infof("Exiting when remaining is empty:  <%v>", remaining)
 			break
 		}
 	}