Merge pull request #104599 from MikeSpreitzer/proper-limit

Adjust LIST work estimator to match current code

Kubernetes-commit: 7997805f339923451d069ba12470bbc79059cf36

go.mod

@@ -45,7 +45,7 @@ require (
 	google.golang.org/grpc v1.38.0
 	gopkg.in/natefinch/lumberjack.v2 v2.0.0
 	gopkg.in/square/go-jose.v2 v2.2.2
-	k8s.io/api v0.0.0-20210825040442-f20796d02069
+	k8s.io/api v0.0.0-20210902155257-9cb86d9d6125
 	k8s.io/apimachinery v0.0.0-20210825040238-74be3b88bedb
 	k8s.io/client-go v0.0.0-20210901080746-a31b18a6ac98
 	k8s.io/component-base v0.0.0-20210901040900-bed685b9e6f5
@@ -58,7 +58,7 @@ require (
 )
 
 replace (
-	k8s.io/api => k8s.io/api v0.0.0-20210825040442-f20796d02069
+	k8s.io/api => k8s.io/api v0.0.0-20210902155257-9cb86d9d6125
 	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210825040238-74be3b88bedb
 	k8s.io/client-go => k8s.io/client-go v0.0.0-20210901080746-a31b18a6ac98
 	k8s.io/component-base => k8s.io/component-base v0.0.0-20210901040900-bed685b9e6f5

go.sum

@@ -939,8 +939,8 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
 honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
-k8s.io/api v0.0.0-20210825040442-f20796d02069 h1:ezx9DT+tM7VpczffbJjnwyr3ulZEaXhE9J5NE4DEHac=
+k8s.io/api v0.0.0-20210902155257-9cb86d9d6125 h1:CVD/hrllBKo7OM5ovOPzsMCM4tZgcEERxOhtg3XXek0=
-k8s.io/api v0.0.0-20210825040442-f20796d02069/go.mod h1:draCg0WF1Lr137sTJ2xg26MplnLcv5PRyIJaP89ntfc=
+k8s.io/api v0.0.0-20210902155257-9cb86d9d6125/go.mod h1:draCg0WF1Lr137sTJ2xg26MplnLcv5PRyIJaP89ntfc=
 k8s.io/apimachinery v0.0.0-20210825040238-74be3b88bedb h1:x865eFVcQlSLu6gCJ4NbCbKGqwgejXJ4yyaZuENYzcQ=
 k8s.io/apimachinery v0.0.0-20210825040238-74be3b88bedb/go.mod h1:YwXrbrp8hgDAA01lpP9HPMC6PS55tjfs3EAatyMMfeM=
 k8s.io/client-go v0.0.0-20210901080746-a31b18a6ac98 h1:1LPKIcr86ISGq5rqn83aw+Mjj/U4beT7FeXS6phRKbA=

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

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",