Merge pull request #133034 from serathius/apf_maxseats

Increase maxSeats for List requests

staging/src/k8s.io/apiserver/pkg/util/flowcontrol/metrics/metrics.go

@@ -326,7 +326,7 @@ var (
 			Help:      "Number of estimated seats (maximum of initial and final seats) associated with requests in API Priority and Fairness",
 			// the upper bound comes from the maximum number of seats a request
 			// can occupy which is currently set at 10.
-			Buckets:        []float64{1, 2, 4, 10},
+			Buckets:        []float64{1, 2, 4, 8, 16, 32, 64, 100},
 			StabilityLevel: compbasemetrics.ALPHA,
 		},
 		[]string{priorityLevel, flowSchema},

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

@@ -20,11 +20,11 @@ import (
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apiserver/pkg/features"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 )
 
 const (
-	minimumSeats                = 1
-	maximumSeatsLimit           = 10
 	objectsPerSeat              = 100.0
 	watchesPerSeat              = 10.0
 	enableMutatingWorkEstimator = true
@@ -40,12 +40,14 @@ type WorkEstimatorConfig struct {
 	// MinimumSeats is the minimum number of seats a request must occupy.
 	MinimumSeats uint64 `json:"minimumSeats,omitempty"`
 
-	// MaximumSeatsLimit is an upper limit on the max seats a request can occupy.
-	//
 	// NOTE: work_estimate_seats_samples metric uses the value of maximumSeats
-	// as the upper bound, so when we change maximumSeats we should also
+	// as the upper bound, so when we change maximum seats values below we should also
 	// update the buckets of the metric.
-	MaximumSeatsLimit uint64 `json:"maximumSeatsLimit,omitempty"`
+
+	// MaximumListSeatsLimit is an upper limit on the max seats a list request can occupy.
+	MaximumListSeatsLimit uint64 `json:"maximumListSeatsLimit,omitempty"`
+	// MaximumListSeatsLimit is an upper limit on the max seats a mutating request can occupy.
+	MaximumMutatingSeatsLimit uint64 `json:"maximumMutatingSeatsLimit,omitempty"`
 }
 
 // ListWorkEstimatorConfig holds work estimator parameters related to list requests.
@@ -65,9 +67,15 @@ type MutatingWorkEstimatorConfig struct {
 
 // DefaultWorkEstimatorConfig creates a new WorkEstimatorConfig with default values.
 func DefaultWorkEstimatorConfig() *WorkEstimatorConfig {
+	var maximumListSeatsLimit uint64 = 10
+	if utilfeature.DefaultFeatureGate.Enabled(features.SizeBasedListCostEstimate) {
+		maximumListSeatsLimit = 100
+	}
+
 	return &WorkEstimatorConfig{
-		MinimumSeats:                minimumSeats,
-		MaximumSeatsLimit:           maximumSeatsLimit,
+		MinimumSeats:                1,
+		MaximumListSeatsLimit:       maximumListSeatsLimit,
+		MaximumMutatingSeatsLimit:   10,
 		ListWorkEstimatorConfig:     defaultListWorkEstimatorConfig(),
 		MutatingWorkEstimatorConfig: defaultMutatingWorkEstimatorConfig(),
 	}

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

@@ -53,8 +53,8 @@ type listWorkEstimator struct {
 func (e *listWorkEstimator) estimate(r *http.Request, flowSchemaName, priorityLevelName string) WorkEstimate {
 	minSeats := e.config.MinimumSeats
 	maxSeats := e.maxSeatsFn(priorityLevelName)
-	if maxSeats == 0 || maxSeats > e.config.MaximumSeatsLimit {
-		maxSeats = e.config.MaximumSeatsLimit
+	if maxSeats == 0 || maxSeats > e.config.MaximumListSeatsLimit {
+		maxSeats = e.config.MaximumListSeatsLimit
 	}
 
 	requestInfo, ok := apirequest.RequestInfoFrom(r.Context())

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

@@ -43,8 +43,8 @@ type mutatingWorkEstimator struct {
 func (e *mutatingWorkEstimator) estimate(r *http.Request, flowSchemaName, priorityLevelName string) WorkEstimate {
 	minSeats := e.config.MinimumSeats
 	maxSeats := e.maxSeatsFn(priorityLevelName)
-	if maxSeats == 0 || maxSeats > e.config.MaximumSeatsLimit {
-		maxSeats = e.config.MaximumSeatsLimit
+	if maxSeats == 0 || maxSeats > e.config.MaximumMutatingSeatsLimit {
+		maxSeats = e.config.MaximumMutatingSeatsLimit
 	}
 
 	// TODO(wojtekt): Remove once we tune the algorithm to not fail

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

@@ -74,8 +74,9 @@ type maxSeatsFunc func(priorityLevelName string) uint64
 // work estimate of 1 seat is allocated to the request.
 func NewWorkEstimator(objectCountFn statsGetterFunc, watchCountFn watchCountGetterFunc, config *WorkEstimatorConfig, maxSeatsFn maxSeatsFunc) WorkEstimatorFunc {
 	estimator := &workEstimator{
+		maxSeatsFn:            maxSeatsFn,
 		minimumSeats:          config.MinimumSeats,
-		maximumSeatsLimit:     config.MaximumSeatsLimit,
+		maximumSeatsLimit:     max(config.MaximumListSeatsLimit, config.MaximumMutatingSeatsLimit),
 		listWorkEstimator:     newListWorkEstimator(objectCountFn, config, maxSeatsFn).estimate,
 		mutatingWorkEstimator: newMutatingWorkEstimator(watchCountFn, config, maxSeatsFn),
 	}
@@ -92,6 +93,7 @@ func (e WorkEstimatorFunc) EstimateWork(r *http.Request, flowSchemaName, priorit
 }
 
 type workEstimator struct {
+	maxSeatsFn maxSeatsFunc
 	// the minimum number of seats a request must occupy
 	minimumSeats uint64
 	// the default maximum number of seats a request can occupy
@@ -107,7 +109,11 @@ func (e *workEstimator) estimate(r *http.Request, flowSchemaName, priorityLevelN
 	if !ok {
 		klog.ErrorS(fmt.Errorf("no RequestInfo found in context"), "Failed to estimate work for the request", "URI", r.RequestURI)
 		// no RequestInfo should never happen, but to be on the safe side let's return maximumSeats
-		return WorkEstimate{InitialSeats: e.maximumSeatsLimit}
+		maxSeats := e.maxSeatsFn(priorityLevelName)
+		if maxSeats == 0 || maxSeats > e.maximumSeatsLimit {
+			maxSeats = e.maximumSeatsLimit
+		}
+		return WorkEstimate{InitialSeats: maxSeats}
 	}
 
 	switch requestInfo.Verb {

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

@@ -71,8 +71,8 @@ func TestWorkEstimator(t *testing.T) {
 				Resource: "events",
 			},
 			stats:                storage.Stats{ObjectCount: 799, EstimatedAverageObjectSizeBytes: 1_000},
-			maxSeats:             10,
-			initialSeatsExpected: 10,
+			maxSeats:             20,
+			initialSeatsExpected: 20,
 		},
 		{
 			name:       "request verb is list, has limit and resource version is 1",
@@ -194,17 +194,29 @@ func TestWorkEstimator(t *testing.T) {
 			initialSeatsExpected: 8,
 		},
 		{
-			name:       "request verb is list, maximum is capped",
+			name:       "request verb is list, capped by watchcache max",
 			requestURI: "http://server/apis/foo.bar/v1/events?resourceVersion=foo",
 			requestInfo: &apirequest.RequestInfo{
 				Verb:     "list",
 				APIGroup: "foo.bar",
 				Resource: "events",
 			},
-			stats:                storage.Stats{ObjectCount: 1999, EstimatedAverageObjectSizeBytes: 1_000},
-			maxSeats:             10,
+			stats:                storage.Stats{ObjectCount: 5000, EstimatedAverageObjectSizeBytes: 1_000},
+			maxSeats:             20,
 			initialSeatsExpected: 10,
 		},
+		{
+			name:       "request verb is list, maximum is capped",
+			requestURI: "http://server/apis/foo.bar/v1/events?resourceVersion=foo&resourceVersionMatch=Exact",
+			requestInfo: &apirequest.RequestInfo{
+				Verb:     "list",
+				APIGroup: "foo.bar",
+				Resource: "events",
+			},
+			stats:                storage.Stats{ObjectCount: 5000, EstimatedAverageObjectSizeBytes: 1_000},
+			maxSeats:             20,
+			initialSeatsExpected: 20,
+		},
 		{
 			name:       "request verb is list, maximum is capped, lower max seats",
 			requestURI: "http://server/apis/foo.bar/v1/events?resourceVersion=foo",
@@ -239,8 +251,8 @@ func TestWorkEstimator(t *testing.T) {
 			},
 			stats:                storage.Stats{ObjectCount: 1999, EstimatedAverageObjectSizeBytes: 1_000},
 			statsErr:             ObjectCountStaleErr,
-			maxSeats:             10,
-			initialSeatsExpected: 10,
+			maxSeats:             20,
+			initialSeatsExpected: 20,
 		},
 		{
 			name:       "request verb is list, object count is not found",
@@ -263,8 +275,8 @@ func TestWorkEstimator(t *testing.T) {
 				Resource: "events",
 			},
 			statsErr:             errors.New("unknown error"),
-			maxSeats:             10,
-			initialSeatsExpected: 10,
+			maxSeats:             20,
+			initialSeatsExpected: 20,
 		},
 		{
 			name:       "request verb is list, metadata.name specified",
@@ -289,7 +301,7 @@ func TestWorkEstimator(t *testing.T) {
 				Resource: "events",
 			},
 			stats:                storage.Stats{ObjectCount: 799, EstimatedAverageObjectSizeBytes: 1_000_000},
-			maxSeats:             10,
+			maxSeats:             20,
 			initialSeatsExpected: 10,
 		},
 		{
@@ -314,7 +326,7 @@ func TestWorkEstimator(t *testing.T) {
 				Resource: "events",
 			},
 			stats:                storage.Stats{ObjectCount: 799, EstimatedAverageObjectSizeBytes: 1_000},
-			initialSeatsExpected: minimumSeats,
+			initialSeatsExpected: 1,
 		},
 		{
 			name:       "request verb is watch, sendInitialEvents is false",
@@ -325,7 +337,7 @@ func TestWorkEstimator(t *testing.T) {
 				Resource: "events",
 			},
 			stats:                storage.Stats{ObjectCount: 799, EstimatedAverageObjectSizeBytes: 1_000},
-			initialSeatsExpected: minimumSeats,
+			initialSeatsExpected: 1,
 		},
 		{
 			name:       "request verb is watch, sendInitialEvents is true",
@@ -380,7 +392,7 @@ func TestWorkEstimator(t *testing.T) {
 			additionalLatencyExpected: 0,
 		},
 		{
-			name:       "request verb is create, watches registered, maximum is capped",
+			name:       "request verb is create, watches registered, capped by watch cache",
 			requestURI: "http://server/apis/foo.bar/v1/foos",
 			requestInfo: &apirequest.RequestInfo{
 				Verb:     "create",
@@ -512,7 +524,7 @@ func TestWorkEstimator(t *testing.T) {
 				Resource: "serviceaccounts",
 			},
 			watchCount:                1000,
-			maxSeats:                  10,
+			maxSeats:                  20,
 			initialSeatsExpected:      1,
 			finalSeatsExpected:        10,
 			additionalLatencyExpected: 50 * time.Millisecond,