From 14a4fd385353b914265acb233fa8d2a426af7f90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Thu, 31 Mar 2022 09:47:41 +0200 Subject: [PATCH] Fix the overestimated cost of deletaged API requests in P&F --- .../flowcontrol/request/list_work_estimator.go | 16 +++++++++++----- .../pkg/util/flowcontrol/request/width_test.go | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go index 7fcc0903e83..50125715c8a 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go @@ -68,17 +68,23 @@ func (e *listWorkEstimator) estimate(r *http.Request, flowSchemaName, priorityLe // pruner will eventually remove the CRD from the cache. return WorkEstimate{InitialSeats: maximumSeats} case err == ObjectCountNotFoundErr: - // there are two scenarios in which we can see this error: + // there are multiple scenarios in which we can see this error: // a. the type is truly unknown, a typo on the caller's part. // b. the count has gone stale for too long and the pruner // has removed the type from the cache. - // we don't have a way to distinguish between a and b. b seems to indicate - // to a more severe case of degradation, although b can naturally trigger - // when a CRD is removed. let's be conservative and allocate maximum seats. - return WorkEstimate{InitialSeats: maximumSeats} + // c. the type is an aggregated resource that is served by a + // different apiserver (thus its object count is not updated) + // we don't have a way to distinguish between those situations. + // However, in case c, the request is delegated to a different apiserver, + // and thus its cost for our server is minimal. To avoid the situation + // when aggregated API calls are overestimated, we allocate the minimum + // possible seats (see #109106 as an example when being more conservative + // led to problems). + return WorkEstimate{InitialSeats: minimumSeats} case err != nil: // we should never be here since Get returns either ObjectCountStaleErr or // ObjectCountNotFoundErr, return maximumSeats to be on the safe side. + klog.ErrorS(err, "Unexpected error from object count tracker") return WorkEstimate{InitialSeats: maximumSeats} } diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/width_test.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/width_test.go index ee27fff42fc..cebe8cb9a66 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/width_test.go +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/width_test.go @@ -125,7 +125,7 @@ func TestWorkEstimator(t *testing.T) { Resource: "events", }, countErr: ObjectCountNotFoundErr, - initialSeatsExpected: maximumSeats, + initialSeatsExpected: minimumSeats, }, { name: "request verb is list, continuation is set", @@ -214,7 +214,7 @@ func TestWorkEstimator(t *testing.T) { Resource: "events", }, countErr: ObjectCountNotFoundErr, - initialSeatsExpected: maximumSeats, + initialSeatsExpected: minimumSeats, }, { name: "request verb is list, object count is stale", @@ -239,7 +239,7 @@ func TestWorkEstimator(t *testing.T) { Resource: "events", }, countErr: ObjectCountNotFoundErr, - initialSeatsExpected: maximumSeats, + initialSeatsExpected: minimumSeats, }, { name: "request verb is list, count getter throws unknown error",