From 396e2d4aa33bb7289cd8e7466e4465f56a73b7d0 Mon Sep 17 00:00:00 2001
From: Aaron Prindle <aprindle@google.com>
Date: Tue, 12 Nov 2019 08:51:49 -0800
Subject: [PATCH] review changes

---
 .../fairqueuing/queueset/queueset.go          |  4 +-
 .../fairqueuing/queueset/queueset_test.go     | 46 +++++++++++--------
 2 files changed, 29 insertions(+), 21 deletions(-)

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 b3d78ef6b99..6862b4ecd77 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
@@ -453,7 +453,7 @@ func (qs *queueSet) selectQueue() *fq.Queue {
 }
 
 // dequeue dequeues a request from the queueSet
-func (qs *queueSet) dequeue() (*fq.Request, bool) {
+func (qs *queueSet) dequeueLocked() (*fq.Request, bool) {
 	queue := qs.selectQueue()
 	if queue == nil {
 		return nil, false
@@ -491,7 +491,7 @@ func (qs *queueSet) dequeueWithChannelAsMuchAsPossible() {
 // require a message to be sent through the requests channel
 // this is a required pattern for the QueueSet the queueSet supports
 func (qs *queueSet) dequeueWithChannel() (*fq.Request, bool) {
-	req, ok := qs.dequeue()
+	req, ok := qs.dequeueLocked()
 	if !ok {
 		return nil, false
 	}
diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset_test.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset_test.go
index 7298984ae50..2b9e30cb963 100644
--- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset_test.go
+++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset_test.go
@@ -32,16 +32,24 @@ import (
 type uniformScenario []uniformClient
 
 type uniformClient struct {
-	hash          uint64
-	nThreads      int
-	nCalls        int
-	execDuration  time.Duration
+	hash     uint64
+	nThreads int
+	nCalls   int
+	// duration for a simulated synchronous call
+	execDuration time.Duration
+	// duration for simulated "other work"
 	thinkDuration time.Duration
 }
 
-// exerciseQueueSetUniformScenario.  Simple logic, only works if each
-// client's offered load is at least as large as its fair share of
-// capacity.
+// exerciseQueueSetUniformScenario runs a scenario based on the given set of uniform clients.
+// Each uniform client specifies a number of threads, each of which alternates between thinking
+// and making a synchronous request through the QueueSet.
+// This function measures how much concurrency each client got, on average, over
+// the intial totalDuration and tests to see whether they all got about the same amount.
+// Each client needs to be demanding enough to use this amount, otherwise the fair result
+// is not equal amounts and the simple test in this function would not accurately test fairness.
+// expectPass indicates whether the QueueSet is expected to be fair.
+// expectedAllRequests indicates whether all requests are expected to get dispatched.
 func exerciseQueueSetUniformScenario(t *testing.T, qs fq.QueueSet, sc uniformScenario,
 	totalDuration time.Duration, expectPass bool, expectedAllRequests bool,
 	clk *clock.FakeEventClock, counter counter.GoRoutineCounter) {
@@ -112,6 +120,18 @@ func exerciseQueueSetUniformScenario(t *testing.T, qs fq.QueueSet, sc uniformSce
 	}
 }
 
+func ClockWait(clk *clock.FakeEventClock, counter counter.GoRoutineCounter, duration time.Duration) {
+	dunch := make(chan struct{})
+	clk.EventAfterDuration(func(time.Time) {
+		counter.Add(1)
+		close(dunch)
+	}, duration)
+	counter.Add(-1)
+	select {
+	case <-dunch:
+	}
+}
+
 // TestNoRestraint should fail because the dummy QueueSet exercises no control
 func TestNoRestraint(t *testing.T) {
 	now := time.Now()
@@ -198,15 +218,3 @@ func TestTimeout(t *testing.T) {
 		{1001001001, 5, 100, time.Second, time.Second},
 	}, time.Second*10, true, false, clk, counter)
 }
-
-func ClockWait(clk *clock.FakeEventClock, counter counter.GoRoutineCounter, duration time.Duration) {
-	dunch := make(chan struct{})
-	clk.EventAfterDuration(func(time.Time) {
-		counter.Add(1)
-		close(dunch)
-	}, duration)
-	counter.Add(-1)
-	select {
-	case <-dunch:
-	}
-}