Merge pull request #104599 from MikeSpreitzer/proper-limit

Adjust LIST work estimator to match current code

staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go

@@ -59,7 +59,7 @@ func (e *listWorkEstimator) estimate(r *http.Request) WorkEstimate {
 	}
 	isListFromCache := !shouldListFromStorage(query, &listOptions)
 
-	count, err := e.countGetterFn(key(requestInfo))
+	numStored, err := e.countGetterFn(key(requestInfo))
 	switch {
 	case err == ObjectCountStaleErr:
 		// object count going stale is indicative of degradation, so we should
@@ -82,23 +82,23 @@ func (e *listWorkEstimator) estimate(r *http.Request) WorkEstimate {
 		return WorkEstimate{Seats: maximumSeats}
 	}
 
-	// TODO: For resources that implement indexes at the watchcache level,
+	limit := numStored
-	//  we need to adjust the cost accordingly
+	if utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) && listOptions.Limit > 0 &&
+		listOptions.Limit < numStored {
+		limit = listOptions.Limit
+	}
+
 	var estimatedObjectsToBeProcessed int64
+
 	switch {
 	case isListFromCache:
-		// if we are here, count is known
+		// TODO: For resources that implement indexes at the watchcache level,
-		estimatedObjectsToBeProcessed = count
+		//  we need to adjust the cost accordingly
+		estimatedObjectsToBeProcessed = numStored
+	case listOptions.FieldSelector != "" || listOptions.LabelSelector != "":
+		estimatedObjectsToBeProcessed = numStored + limit
 	default:
-		// Even if a selector is specified and we may need to list and go over more objects from etcd
+		estimatedObjectsToBeProcessed = 2 * limit
-		// to produce the result of size <limit>, each individual chunk will be of size at most <limit>.
-		// As a result. the work estimate of the request should be computed based on <limit> and the actual
-		// cost of processing more elements will be hidden in the request processing latency.
-		estimatedObjectsToBeProcessed = listOptions.Limit
-		if estimatedObjectsToBeProcessed == 0 {
-			// limit has not been specified, fall back to count
-			estimatedObjectsToBeProcessed = count
-		}
 	}
 
 	// for now, our rough estimate is to allocate one seat to each 100 obejcts that

staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/width_test.go

@@ -60,55 +60,42 @@ func TestWorkEstimator(t *testing.T) {
 			},
 			seatsExpected: maximumSeats,
 		},
+		{
+			name:       "request verb is list, has limit and resource version is 1",
+			requestURI: "http://server/apis/foo.bar/v1/events?limit=399&resourceVersion=1",
+			requestInfo: &apirequest.RequestInfo{
+				Verb:     "list",
+				APIGroup: "foo.bar",
+				Resource: "events",
+			},
+			counts: map[string]int64{
+				"events.foo.bar": 699,
+			},
+			seatsExpected: 8,
+		},
+		{
+			name:       "request verb is list, limit not set",
+			requestURI: "http://server/apis/foo.bar/v1/events?resourceVersion=1",
+			requestInfo: &apirequest.RequestInfo{
+				Verb:     "list",
+				APIGroup: "foo.bar",
+				Resource: "events",
+			},
+			counts: map[string]int64{
+				"events.foo.bar": 699,
+			},
+			seatsExpected: 7,
+		},
 		{
 			name:       "request verb is list, resource version not set",
-			requestURI: "http://server/apis/foo.bar/v1/events?limit=499",
+			requestURI: "http://server/apis/foo.bar/v1/events?limit=399",
 			requestInfo: &apirequest.RequestInfo{
 				Verb:     "list",
 				APIGroup: "foo.bar",
 				Resource: "events",
 			},
 			counts: map[string]int64{
-				"events.foo.bar": 799,
+				"events.foo.bar": 699,
-			},
-			seatsExpected: 5,
-		},
-		{
-			name:       "request verb is list, continuation is set",
-			requestURI: "http://server/apis/foo.bar/v1/events?continue=token&limit=499&resourceVersion=1",
-			requestInfo: &apirequest.RequestInfo{
-				Verb:     "list",
-				APIGroup: "foo.bar",
-				Resource: "events",
-			},
-			counts: map[string]int64{
-				"events.foo.bar": 799,
-			},
-			seatsExpected: 5,
-		},
-		{
-			name:       "request verb is list, has limit",
-			requestURI: "http://server/apis/foo.bar/v1/events?limit=499&resourceVersion=1",
-			requestInfo: &apirequest.RequestInfo{
-				Verb:     "list",
-				APIGroup: "foo.bar",
-				Resource: "events",
-			},
-			counts: map[string]int64{
-				"events.foo.bar": 799,
-			},
-			seatsExpected: 5,
-		},
-		{
-			name:       "request verb is list, resource version is zero",
-			requestURI: "http://server/apis/foo.bar/v1/events?resourceVersion=0",
-			requestInfo: &apirequest.RequestInfo{
-				Verb:     "list",
-				APIGroup: "foo.bar",
-				Resource: "events",
-			},
-			counts: map[string]int64{
-				"events.foo.bar": 799,
 			},
 			seatsExpected: 8,
 		},
@@ -121,7 +108,7 @@ func TestWorkEstimator(t *testing.T) {
 				Resource: "events",
 			},
 			counts: map[string]int64{
-				"events.foo.bar": 799,
+				"events.foo.bar": 399,
 			},
 			seatsExpected: 8,
 		},
@@ -137,8 +124,34 @@ func TestWorkEstimator(t *testing.T) {
 			seatsExpected: maximumSeats,
 		},
 		{
-			name:       "request verb is list, resource version match is Exact",
+			name:       "request verb is list, continuation is set",
-			requestURI: "http://server/apis/foo.bar/v1/events?resourceVersion=foo&resourceVersionMatch=Exact&limit=499",
+			requestURI: "http://server/apis/foo.bar/v1/events?continue=token&limit=399",
+			requestInfo: &apirequest.RequestInfo{
+				Verb:     "list",
+				APIGroup: "foo.bar",
+				Resource: "events",
+			},
+			counts: map[string]int64{
+				"events.foo.bar": 699,
+			},
+			seatsExpected: 8,
+		},
+		{
+			name:       "request verb is list, resource version is zero",
+			requestURI: "http://server/apis/foo.bar/v1/events?limit=299&resourceVersion=0",
+			requestInfo: &apirequest.RequestInfo{
+				Verb:     "list",
+				APIGroup: "foo.bar",
+				Resource: "events",
+			},
+			counts: map[string]int64{
+				"events.foo.bar": 399,
+			},
+			seatsExpected: 4,
+		},
+		{
+			name:       "request verb is list, resource version is zero, no limit",
+			requestURI: "http://server/apis/foo.bar/v1/events?resourceVersion=0",
 			requestInfo: &apirequest.RequestInfo{
 				Verb:     "list",
 				APIGroup: "foo.bar",
@@ -147,7 +160,20 @@ func TestWorkEstimator(t *testing.T) {
 			counts: map[string]int64{
 				"events.foo.bar": 799,
 			},
-			seatsExpected: 5,
+			seatsExpected: 8,
+		},
+		{
+			name:       "request verb is list, resource version match is Exact",
+			requestURI: "http://server/apis/foo.bar/v1/events?resourceVersion=foo&resourceVersionMatch=Exact&limit=399",
+			requestInfo: &apirequest.RequestInfo{
+				Verb:     "list",
+				APIGroup: "foo.bar",
+				Resource: "events",
+			},
+			counts: map[string]int64{
+				"events.foo.bar": 699,
+			},
+			seatsExpected: 8,
 		},
 		{
 			name:       "request verb is list, resource version match is NotOlderThan, limit not specified",