From cd06ba502cf85603242008cc9f61234790ede6de Mon Sep 17 00:00:00 2001
From: Abu Kashem <akashem@redhat.com>
Date: Mon, 18 Oct 2021 12:46:54 -0400
Subject: [PATCH] apf: return nil for a request that has been removed from
 queue

---
 .../flowcontrol/fairqueuing/queueset/fifo_list.go | 12 +++++++-----
 .../fairqueuing/queueset/fifo_list_test.go        | 15 +++++++++++++++
 .../flowcontrol/fairqueuing/queueset/queueset.go  | 11 ++++++-----
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/fifo_list.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/fifo_list.go
index 9a5fc77b072..3b22a12e1a1 100644
--- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/fifo_list.go
+++ b/staging/src/k8s.io/apiserver/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 {
-			l.Remove(e)
-			e.Value = nil
-			deductFromQueueSum(&l.sum, req)
+		if e.Value == nil {
+			return nil
 		}
+		l.Remove(e)
+		e.Value = nil
+		deductFromQueueSum(&l.sum, req)
 		return req
 	}
 }
diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/fifo_list_test.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/fifo_list_test.go
index c3490910bcd..22cb2ca7068 100644
--- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/fifo_list_test.go
+++ b/staging/src/k8s.io/apiserver/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()
 
diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset.go
index d2023d8faea..1dfd7d5757f 100644
--- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset.go
+++ b/staging/src/k8s.io/apiserver/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()
-	qs.totRequestsWaiting--
-	metrics.AddRequestsInQueues(req.ctx, qs.qCfg.Name, req.fsName, -1)
-	req.NoteQueued(false)
-	qs.obsPair.RequestsWaiting.Add(-1)
+	if req.removeFromQueueLocked() != nil {
+		qs.totRequestsWaiting--
+		metrics.AddRequestsInQueues(req.ctx, qs.qCfg.Name, req.fsName, -1)
+		req.NoteQueued(false)
+		qs.obsPair.RequestsWaiting.Add(-1)
+	}
 	return false, qs.isIdleLocked()
 }