Merge pull request #46234 from wojtek-t/faster_selflink

Automatic merge from submit-queue (batch tested with PRs 46060, 46234)

Speedup generating selflinks for list and watch requests

I've seen profiles, where GenerateSelflink was 8-9% of whole cpu usage of apiserver (profiles over 30s). Most of this where spent in getting RequestInfo from the context and creating the context.

This PR changes the API of the GenerateLink method of the namer which results in computing the context and requestInfo only once per LIST/WATCH request (instead of computing it for every single returned element of LIST/WATCH).

@smarterclayton @deads2k - can one of you please take a look?

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go

@@ -45,7 +45,7 @@ type ScopeNamer interface {
 	SetSelfLink(obj runtime.Object, url string) error
 	// GenerateLink creates an encoded URI for a given runtime object that represents the canonical path
 	// and query.
-	GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error)
+	GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error)
 	// GenerateListLink creates an encoded URI for a list that represents the canonical path and query.
 	GenerateListLink(req *http.Request) (uri string, err error)
 }
@@ -90,12 +90,7 @@ func (n ContextBasedNaming) Name(req *http.Request) (namespace, name string, err
 	return ns, requestInfo.Name, nil
 }
 
-func (n ContextBasedNaming) GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error) {
+func (n ContextBasedNaming) GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) {
-	requestInfo, ok := request.RequestInfoFrom(n.GetContext(req))
-	if !ok {
-		return "", fmt.Errorf("missing requestInfo")
-	}
-
 	namespace, name, err := n.ObjectName(obj)
 	if err == errEmptyName && len(requestInfo.Name) > 0 {
 		name = requestInfo.Name

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go

@@ -102,7 +102,12 @@ func getResourceHandler(scope RequestScope, getter getterFunc) http.HandlerFunc
 			scope.err(err, w, req)
 			return
 		}
-		if err := setSelfLink(result, req, scope.Namer); err != nil {
+		requestInfo, ok := request.RequestInfoFrom(ctx)
+		if !ok {
+			scope.err(fmt.Errorf("missing requestInfo"), w, req)
+			return
+		}
+		if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
 			scope.err(err, w, req)
 			return
 		}
@@ -321,7 +326,7 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch
 			return
 		}
 		trace.Step("Listing from storage done")
-		numberOfItems, err := setListSelfLink(result, req, scope.Namer)
+		numberOfItems, err := setListSelfLink(result, ctx, req, scope.Namer)
 		if err != nil {
 			scope.err(err, w, req)
 			return
@@ -419,7 +424,12 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object
 		}
 		trace.Step("Object stored in database")
 
-		if err := setSelfLink(result, req, scope.Namer); err != nil {
+		requestInfo, ok := request.RequestInfoFrom(ctx)
+		if !ok {
+			scope.err(fmt.Errorf("missing requestInfo"), w, req)
+			return
+		}
+		if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
 			scope.err(err, w, req)
 			return
 		}
@@ -512,7 +522,12 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
 			return
 		}
 
-		if err := setSelfLink(result, req, scope.Namer); err != nil {
+		requestInfo, ok := request.RequestInfoFrom(ctx)
+		if !ok {
+			scope.err(fmt.Errorf("missing requestInfo"), w, req)
+			return
+		}
+		if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
 			scope.err(err, w, req)
 			return
 		}
@@ -829,7 +844,12 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType
 		}
 		trace.Step("Object stored in database")
 
-		if err := setSelfLink(result, req, scope.Namer); err != nil {
+		requestInfo, ok := request.RequestInfoFrom(ctx)
+		if !ok {
+			scope.err(fmt.Errorf("missing requestInfo"), w, req)
+			return
+		}
+		if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
 			scope.err(err, w, req)
 			return
 		}
@@ -942,8 +962,13 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
 			}
 		} else {
 			// when a non-status response is returned, set the self link
+			requestInfo, ok := request.RequestInfoFrom(ctx)
+			if !ok {
+				scope.err(fmt.Errorf("missing requestInfo"), w, req)
+				return
+			}
 			if _, ok := result.(*metav1.Status); !ok {
-				if err := setSelfLink(result, req, scope.Namer); err != nil {
+				if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
 					scope.err(err, w, req)
 					return
 				}
@@ -1045,7 +1070,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
 		} else {
 			// when a non-status response is returned, set the self link
 			if _, ok := result.(*metav1.Status); !ok {
-				if _, err := setListSelfLink(result, req, scope.Namer); err != nil {
+				if _, err := setListSelfLink(result, ctx, req, scope.Namer); err != nil {
 					scope.err(err, w, req)
 					return
 				}
@@ -1113,9 +1138,9 @@ func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime
 
 // setSelfLink sets the self link of an object (or the child items in a list) to the base URL of the request
 // plus the path and query generated by the provided linkFunc
-func setSelfLink(obj runtime.Object, req *http.Request, namer ScopeNamer) error {
+func setSelfLink(obj runtime.Object, requestInfo *request.RequestInfo, namer ScopeNamer) error {
 	// TODO: SelfLink generation should return a full URL?
-	uri, err := namer.GenerateLink(req, obj)
+	uri, err := namer.GenerateLink(requestInfo, obj)
 	if err != nil {
 		return nil
 	}
@@ -1160,7 +1185,7 @@ func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) err
 
 // setListSelfLink sets the self link of a list to the base URL, then sets the self links
 // on all child objects returned. Returns the number of items in the list.
-func setListSelfLink(obj runtime.Object, req *http.Request, namer ScopeNamer) (int, error) {
+func setListSelfLink(obj runtime.Object, ctx request.Context, req *http.Request, namer ScopeNamer) (int, error) {
 	if !meta.IsListType(obj) {
 		return 0, nil
 	}
@@ -1172,11 +1197,15 @@ func setListSelfLink(obj runtime.Object, req *http.Request, namer ScopeNamer) (i
 	if err := namer.SetSelfLink(obj, uri); err != nil {
 		glog.V(4).Infof("Unable to set self link on object: %v", err)
 	}
+	requestInfo, ok := request.RequestInfoFrom(ctx)
+	if !ok {
+		return 0, fmt.Errorf("missing requestInfo")
+	}
 
 	count := 0
 	err = meta.EachListItem(obj, func(obj runtime.Object) error {
 		count++
-		return setSelfLink(obj, req, namer)
+		return setSelfLink(obj, requestInfo, namer)
 	})
 	return count, err
 }

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go

@@ -160,11 +160,11 @@ func (p *testNamer) SetSelfLink(obj runtime.Object, url string) error {
 }
 
 // GenerateLink creates a path and query for a given runtime object that represents the canonical path.
-func (p *testNamer) GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error) {
+func (p *testNamer) GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) {
 	return "", errors.New("not implemented")
 }
 
-// GenerateLink creates a path and query for a list that represents the canonical path.
+// GenerateListLink creates a path and query for a list that represents the canonical path.
 func (p *testNamer) GenerateListLink(req *http.Request) (uri string, err error) {
 	return "", errors.New("not implemented")
 }

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go

@@ -30,6 +30,7 @@ import (
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
+	"k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/server/httplog"
 	"k8s.io/apiserver/pkg/util/wsstream"
 
@@ -88,6 +89,13 @@ func serveWatch(watcher watch.Interface, scope RequestScope, req *http.Request,
 		mediaType += ";stream=watch"
 	}
 
+	ctx := scope.ContextFunc(req)
+	requestInfo, ok := request.RequestInfoFrom(ctx)
+	if !ok {
+		scope.err(fmt.Errorf("missing requestInfo"), w, req)
+		return
+	}
+
 	server := &WatchServer{
 		Watching: watcher,
 		Scope:    scope,
@@ -98,7 +106,7 @@ func serveWatch(watcher watch.Interface, scope RequestScope, req *http.Request,
 		Encoder:         encoder,
 		EmbeddedEncoder: embeddedEncoder,
 		Fixup: func(obj runtime.Object) {
-			if err := setSelfLink(obj, req, scope.Namer); err != nil {
+			if err := setSelfLink(obj, requestInfo, scope.Namer); err != nil {
 				utilruntime.HandleError(fmt.Errorf("failed to set link for object %v: %v", reflect.TypeOf(obj), err))
 			}
 		},