Merge pull request #105085 from MikeSpreitzer/fix-queueset-tests

Update TestNoRestraint and TestWindup

staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset_test.go

@@ -413,29 +413,42 @@ func TestMain(m *testing.M) {
 }
 
 // TestNoRestraint tests whether the no-restraint factory gives every client what it asks for
+// even though that is unfair.
 func TestNoRestraint(t *testing.T) {
 	metrics.Register()
-	now := time.Now()
+	testCases := []struct {
-	clk, counter := testeventclock.NewFake(now, 0, nil)
+		concurrency int
-	nrc, err := test.NewNoRestraintFactory().BeginConstruction(fq.QueuingConfig{}, newObserverPair(clk))
+		fair        bool
-	if err != nil {
+	}{
-		t.Fatal(err)
+		{concurrency: 10, fair: true},
+		{concurrency: 2, fair: false},
+	}
+	for _, testCase := range testCases {
+		subName := fmt.Sprintf("concurrency=%v", testCase.concurrency)
+		t.Run(subName, func(t *testing.T) {
+			now := time.Now()
+			clk, counter := testeventclock.NewFake(now, 0, nil)
+			nrc, err := test.NewNoRestraintFactory().BeginConstruction(fq.QueuingConfig{}, newObserverPair(clk))
+			if err != nil {
+				t.Fatal(err)
+			}
+			nr := nrc.Complete(fq.DispatchingConfig{})
+			uniformScenario{name: "NoRestraint/" + subName,
+				qs: nr,
+				clients: []uniformClient{
+					newUniformClient(1001001001, 5, 10, time.Second, time.Second),
+					newUniformClient(2002002002, 2, 10, time.Second, time.Second/2),
+				},
+				concurrencyLimit:       testCase.concurrency,
+				evalDuration:           time.Second * 15,
+				expectedFair:           []bool{testCase.fair},
+				expectedFairnessMargin: []float64{0.1},
+				expectAllRequests:      true,
+				clk:                    clk,
+				counter:                counter,
+			}.exercise(t)
+		})
 	}
-	nr := nrc.Complete(fq.DispatchingConfig{})
-	uniformScenario{name: "NoRestraint",
-		qs: nr,
-		clients: []uniformClient{
-			newUniformClient(1001001001, 5, 10, time.Second, time.Second),
-			newUniformClient(2002002002, 2, 10, time.Second, time.Second/2),
-		},
-		concurrencyLimit:       10,
-		evalDuration:           time.Second * 15,
-		expectedFair:           []bool{true},
-		expectedFairnessMargin: []float64{0.1},
-		expectAllRequests:      true,
-		clk:                    clk,
-		counter:                counter,
-	}.exercise(t)
 }
 
 func TestBaseline(t *testing.T) {
@@ -733,9 +746,9 @@ func TestTooWide(t *testing.T) {
 			newUniformClient(90090090090090, 15, 21, time.Second, time.Second-1).seats(7),
 		},
 		concurrencyLimit:       6,
-		evalDuration:           time.Second * 40,
+		evalDuration:           time.Second * 435,
 		expectedFair:           []bool{true},
-		expectedFairnessMargin: []float64{0.38},
+		expectedFairnessMargin: []float64{0.375},
 		expectAllRequests:      true,
 		evalInqueueMetrics:     true,
 		evalExecutingMetrics:   true,
@@ -744,40 +757,64 @@ func TestTooWide(t *testing.T) {
 	}.exercise(t)
 }
 
+// TestWindup exercises a scenario with the windup problem.
+// That is, a flow that can not use all the seats that it is allocated
+// for a while.  During that time, the queues that serve that flow
+// advance their `virtualStart` (that is, R(next dispatch in virtual world))
+// more slowly than the other queues (which are using more seats than they
+// are allocated).  The implementation has a hack that addresses part of
+// this imbalance but not all of it.  In this test, flow 1 can not use all
+// of its allocation during the first half, and *can* (and does) use all of
+// its allocation and more during the second half.
+// Thus we expect the fair (not equal) result
+// in the first half and an unfair result in the second half.
+// This func has two test cases, bounding the amount of unfairness
+// in the second half.
 func TestWindup(t *testing.T) {
 	metrics.Register()
-	now := time.Now()
+	testCases := []struct {
-
+		margin2     float64
-	clk, counter := testeventclock.NewFake(now, 0, nil)
+		expectFair2 bool
-	qsf := newTestableQueueSetFactory(clk, countingPromiseFactoryFactory(counter))
+	}{
-	qCfg := fq.QueuingConfig{
+		{margin2: 0.26, expectFair2: true},
-		Name:             "TestWindup",
+		{margin2: 0.1, expectFair2: false},
-		DesiredNumQueues: 9,
-		QueueLengthLimit: 6,
-		HandSize:         1,
-		RequestWaitLimit: 10 * time.Minute,
 	}
-	qsc, err := qsf.BeginConstruction(qCfg, newObserverPair(clk))
+	for _, testCase := range testCases {
-	if err != nil {
+		subName := fmt.Sprintf("m2=%v", testCase.margin2)
-		t.Fatal(err)
+		t.Run(subName, func(t *testing.T) {
-	}
+			now := time.Now()
-	qs := qsc.Complete(fq.DispatchingConfig{ConcurrencyLimit: 3})
+			clk, counter := testeventclock.NewFake(now, 0, nil)
+			qsf := newTestableQueueSetFactory(clk, countingPromiseFactoryFactory(counter))
+			qCfg := fq.QueuingConfig{
+				Name:             "TestWindup/" + subName,
+				DesiredNumQueues: 9,
+				QueueLengthLimit: 6,
+				HandSize:         1,
+				RequestWaitLimit: 10 * time.Minute,
+			}
+			qsc, err := qsf.BeginConstruction(qCfg, newObserverPair(clk))
+			if err != nil {
+				t.Fatal(err)
+			}
+			qs := qsc.Complete(fq.DispatchingConfig{ConcurrencyLimit: 3})
 
-	uniformScenario{name: qCfg.Name, qs: qs,
+			uniformScenario{name: qCfg.Name, qs: qs,
-		clients: []uniformClient{
+				clients: []uniformClient{
-			newUniformClient(1001001001, 2, 40, time.Second, -1),
+					newUniformClient(1001001001, 2, 40, time.Second, -1),
-			newUniformClient(2002002002, 2, 40, time.Second, -1).setSplit(),
+					newUniformClient(2002002002, 2, 40, time.Second, -1).setSplit(),
-		},
+				},
-		concurrencyLimit:       3,
+				concurrencyLimit:       3,
-		evalDuration:           time.Second * 40,
+				evalDuration:           time.Second * 40,
-		expectedFair:           []bool{true, true},
+				expectedFair:           []bool{true, testCase.expectFair2},
-		expectedFairnessMargin: []float64{0.1, 0.26},
+				expectedFairnessMargin: []float64{0.1, testCase.margin2},
-		expectAllRequests:      true,
+				expectAllRequests:      true,
-		evalInqueueMetrics:     true,
+				evalInqueueMetrics:     true,
-		evalExecutingMetrics:   true,
+				evalExecutingMetrics:   true,
-		clk:                    clk,
+				clk:                    clk,
-		counter:                counter,
+				counter:                counter,
-	}.exercise(t)
+			}.exercise(t)
+		})
+	}
 }
 
 func TestDifferentFlowsWithoutQueuing(t *testing.T) {