apf: return nil for a request that has been removed from queue

Kubernetes-commit: cd06ba502cf85603242008cc9f61234790ede6de

pkg/util/flowcontrol/fairqueuing/queueset/fifo_list.go

@@ -22,7 +22,8 @@ import (
 
 // removeFromFIFOFunc removes a designated element from the list.
 // The complexity of the runtime cost is O(1)
-// It returns the request removed from the list.
+// It returns the request that has been removed from the list,
+// it returns nil if the request has already been removed.
 type removeFromFIFOFunc func() *request
 
 // walkFunc is called for each request in the list in the
@@ -89,11 +90,12 @@ func (l *requestFIFO) Enqueue(req *request) removeFromFIFOFunc {
 	addToQueueSum(&l.sum, req)
 
 	return func() *request {
-		if e.Value != nil {
+		if e.Value == nil {
-			l.Remove(e)
+			return nil
-			e.Value = nil
-			deductFromQueueSum(&l.sum, req)
 		}
+		l.Remove(e)
+		e.Value = nil
+		deductFromQueueSum(&l.sum, req)
 		return req
 	}
 }

pkg/util/flowcontrol/fairqueuing/queueset/fifo_list_test.go

@@ -100,6 +100,21 @@ func TestFIFOWithRemoveMultipleRequestsInArrivalOrder(t *testing.T) {
 	verifyOrder(t, arrival, dequeued)
 }
 
+func TestFIFORemoveFromFIFOFunc(t *testing.T) {
+	list := newRequestFIFO()
+	reqWant := &request{}
+	removeFn := list.Enqueue(reqWant)
+
+	reqGot := removeFn()
+	if reqWant != reqGot {
+		t.Errorf("Expected request identity: %p, but got: %p)", reqWant, reqGot)
+	}
+
+	if got := removeFn(); got != nil {
+		t.Errorf("Expected a nil request, but got: %v)", got)
+	}
+}
+
 func TestFIFOWithRemoveMultipleRequestsInRandomOrder(t *testing.T) {
 	list := newRequestFIFO()
 

pkg/util/flowcontrol/fairqueuing/queueset/queueset.go

@@ -386,11 +386,12 @@ func (req *request) wait() (bool, bool) {
 	// TODO(aaron-prindle) add metrics for this case
 	klog.V(5).Infof("QS(%s): Ejecting request %#+v %#+v from its queue", qs.qCfg.Name, req.descr1, req.descr2)
 	// remove the request from the queue as it has timed out
-	req.removeFromQueueLocked()
+	if req.removeFromQueueLocked() != nil {
-	qs.totRequestsWaiting--
+		qs.totRequestsWaiting--
-	metrics.AddRequestsInQueues(req.ctx, qs.qCfg.Name, req.fsName, -1)
+		metrics.AddRequestsInQueues(req.ctx, qs.qCfg.Name, req.fsName, -1)
-	req.NoteQueued(false)
+		req.NoteQueued(false)
-	qs.obsPair.RequestsWaiting.Add(-1)
+		qs.obsPair.RequestsWaiting.Add(-1)
+	}
 	return false, qs.isIdleLocked()
 }