Removed timer, keeping only config change

Author: mani

pkg/scheduler/objects/queue.go

@@ -89,7 +89,6 @@ type Queue struct {
 	template                   *template.Template
 	queueEvents                *schedEvt.QueueEvents
 	quotaChangePreemptionDelay uint64
-	quotaChangePreemptionTimer *time.Timer
 
 	locking.RWMutex
 }
@@ -113,7 +112,6 @@ func newBlankQueue() *Queue {
 		preemptionDelay:            configs.DefaultPreemptionDelay,
 		preemptionPolicy:           policies.DefaultPreemptionPolicy,
 		quotaChangePreemptionDelay: 0,
-		quotaChangePreemptionTimer: nil,
 	}
 }
 
@@ -409,37 +407,30 @@ func (sq *Queue) setPreemptionSettings(oldMaxResource *resources.Resource, conf
 		// Quota decrease
 		case resources.StrictlyGreaterThan(oldMaxResource, newMaxResource) && conf.Preemption.Delay != 0:
 			set = true
+			// Quota increase
+		case resources.StrictlyGreaterThan(newMaxResource, oldMaxResource) && conf.Preemption.Delay != 0:
+			reset = true
 			// Quota remains as is but delay has changed
 		case resources.Equals(oldMaxResource, newMaxResource) && conf.Preemption.Delay != 0 && sq.quotaChangePreemptionDelay != conf.Preemption.Delay:
 			sq.quotaChangePreemptionDelay = conf.Preemption.Delay
-			sq.quotaChangePreemptionTimer.Reset(time.Duration(sq.quotaChangePreemptionDelay))
-		// Quota increase
 		default:
-			reset = true
+			// noop
 		}
 	}
 
 	// Set preemption settings
 	if set {
 		sq.quotaChangePreemptionDelay = conf.Preemption.Delay
-		sq.quotaChangePreemptionTimer = time.AfterFunc(time.Duration(sq.quotaChangePreemptionDelay), sq.tryPreemptionToEnforceQuota)
 	}
 
 	// Reset preemption settings
 	if reset {
 		if sq.quotaChangePreemptionDelay != 0 {
 			sq.quotaChangePreemptionDelay = 0
-			sq.quotaChangePreemptionTimer.Stop()
-			sq.quotaChangePreemptionTimer = nil
 		}
 	}
 }
 
-// tryPreemptionToEnforceQuota Try Preemption to enforce quota change if the quota change preemption delay is reached
-func (sq *Queue) tryPreemptionToEnforceQuota() {
-
-}
-
 // setResourcesFromConf sets the maxResource and guaranteedResource of the queue from the config.
 func (sq *Queue) setResourcesFromConf(resource configs.Resources) error {
 	maxResource, err := resources.NewResourceFromConf(resource.Max)

pkg/scheduler/objects/queue_test.go

@@ -2250,63 +2250,64 @@ func TestQuotaChangePreemptionSettings(t *testing.T) {
 
 	parent, err := createManagedQueueWithProps(root, "parent", false, getResourceConf(), getResourceConf())
 	assert.NilError(t, err, "failed to create basic queue: %v", err)
-
-	var nilTimer *time.Timer
 	testCases := []struct {
 		name          string
 		conf          configs.QueueConfig
 		expectedDelay uint64
-		expectedTimer *time.Timer
 	}{{"first queue setup without delay", configs.QueueConfig{
 		Resources: configs.Resources{
 			Max:        getResourceConf(),
 			Guaranteed: getResourceConf(),
 		},
-	}, 0, nilTimer},
+	}, 0},
 		{"increase max with delay", configs.QueueConfig{
 			Resources: configs.Resources{
 				Max: map[string]string{"memory": "100000000"},
 			},
 			Preemption: configs.Preemption{
 				Delay: 500,
 			},
-		}, 0, nilTimer},
+		}, 0},
 		{"decrease max with delay", configs.QueueConfig{
 			Resources: configs.Resources{
 				Max: map[string]string{"memory": "100"},
 			},
 			Preemption: configs.Preemption{
 				Delay: 500,
 			},
-		}, 500, time.AfterFunc(time.Duration(500), parent.tryPreemptionToEnforceQuota)},
+		}, 500},
 		{"max remains as is but delay changed", configs.QueueConfig{
 			Resources: configs.Resources{
 				Max: map[string]string{"memory": "100"},
 			},
 			Preemption: configs.Preemption{
 				Delay: 200,
 			},
-		}, 200, time.AfterFunc(time.Duration(200), parent.tryPreemptionToEnforceQuota)},
+		}, 200},
+		{"unrelated config change, should not impact earlier set preemption settings", configs.QueueConfig{
+			Resources: configs.Resources{
+				Max:        map[string]string{"memory": "100"},
+				Guaranteed: map[string]string{"memory": "50"},
+			},
+			Preemption: configs.Preemption{
+				Delay: 200,
+			},
+		}, 200},
 		{"increase max again with delay", configs.QueueConfig{
 			Resources: configs.Resources{
 				Max: map[string]string{"memory": "101"},
 			},
 			Preemption: configs.Preemption{
 				Delay: 200,
 			},
-		}, 0, nilTimer}}
+		}, 0}}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			err = parent.ApplyConf(tc.conf)
 			assert.NilError(t, err, "failed to apply conf: %v", err)
 			assert.Assert(t, parent.maxResource != nil)
 			assert.Equal(t, parent.quotaChangePreemptionDelay, tc.expectedDelay)
-			if tc.expectedTimer != nil {
-				assert.Equal(t, *parent.quotaChangePreemptionTimer, *tc.expectedTimer)
-			} else {
-				assert.Assert(t, parent.quotaChangePreemptionTimer == nil)
-			}
 		})
 	}
 }
@@ -2341,8 +2342,6 @@ func TestNewConfiguredQueue(t *testing.T) {
 	assert.Assert(t, resources.Equals(resourceStruct, parent.template.GetMaxResource()))
 	assert.Assert(t, resources.Equals(resourceStruct, parent.template.GetGuaranteedResource()))
 	assert.Equal(t, parent.quotaChangePreemptionDelay, uint64(0))
-	var expectedTimer *time.Timer
-	assert.Equal(t, parent.quotaChangePreemptionTimer, expectedTimer)
 
 	// case 0: managed leaf queue can't use template
 	leafConfig := configs.QueueConfig{
@@ -2369,8 +2368,6 @@ func TestNewConfiguredQueue(t *testing.T) {
 	assert.NilError(t, err, "Resource creation failed")
 	assert.Assert(t, resources.Equals(childLeaf.guaranteedResource, childLeafGuaranteed))
 	assert.Equal(t, childLeaf.quotaChangePreemptionDelay, uint64(500))
-	leafExpectedTimer := time.AfterFunc(time.Duration(uint64(500)), childLeaf.tryPreemptionToEnforceQuota)
-	assert.Equal(t, *childLeaf.quotaChangePreemptionTimer, *leafExpectedTimer)
 
 	// case 1: non-leaf can't use template but it can inherit template from parent
 	NonLeafConfig := configs.QueueConfig{
@@ -2388,8 +2385,6 @@ func TestNewConfiguredQueue(t *testing.T) {
 	assert.Assert(t, childNonLeaf.guaranteedResource == nil)
 	assert.Assert(t, childNonLeaf.maxResource == nil)
 	assert.Equal(t, childNonLeaf.quotaChangePreemptionDelay, uint64(0))
-	var nonLeafExpectedTimer *time.Timer
-	assert.Equal(t, childNonLeaf.quotaChangePreemptionTimer, nonLeafExpectedTimer)
 
 	// case 2: do not send queue event when silence flag is set to true
 	events.Init()
@@ -2407,8 +2402,6 @@ func TestNewConfiguredQueue(t *testing.T) {
 	noEvents := eventSystem.Store.CountStoredEvents()
 	assert.Equal(t, noEvents, uint64(0), "expected 0 event, got %d", noEvents)
 	assert.Equal(t, rootQ.quotaChangePreemptionDelay, uint64(0))
-	var rootQTimer *time.Timer
-	assert.Equal(t, rootQ.quotaChangePreemptionTimer, rootQTimer)
 }
 
 func TestResetRunningState(t *testing.T) {