Merge pull request #75570 from smarterclayton/fix_performance

Fix a regression in watch performance and reduce allocations in the GET path
This commit is contained in:
Kubernetes Prow Robot 2019-03-22 11:42:51 -07:00 committed by GitHub
commit b3824cd094
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 211 additions and 71 deletions

View File

@ -644,6 +644,8 @@ func (s unstructuredNegotiatedSerializer) SupportedMediaTypes() []runtime.Serial
return []runtime.SerializerInfo{
{
MediaType: "application/json",
MediaTypeType: "application",
MediaTypeSubType: "json",
EncodesAsText: true,
Serializer: json.NewSerializer(json.DefaultMetaFactory, s.creator, s.typer, false),
PrettySerializer: json.NewSerializer(json.DefaultMetaFactory, s.creator, s.typer, true),
@ -654,9 +656,11 @@ func (s unstructuredNegotiatedSerializer) SupportedMediaTypes() []runtime.Serial
},
},
{
MediaType: "application/yaml",
EncodesAsText: true,
Serializer: json.NewYAMLSerializer(json.DefaultMetaFactory, s.creator, s.typer),
MediaType: "application/yaml",
MediaTypeType: "application",
MediaTypeSubType: "yaml",
EncodesAsText: true,
Serializer: json.NewYAMLSerializer(json.DefaultMetaFactory, s.creator, s.typer),
},
}
}

View File

@ -17,30 +17,76 @@ limitations under the License.
package meta
import (
"errors"
"fmt"
"reflect"
"sync"
"k8s.io/apimachinery/pkg/conversion"
"k8s.io/apimachinery/pkg/runtime"
)
// IsListType returns true if the provided Object has a slice called Items
var (
// isListCache maintains a cache of types that are checked for lists
// which is used by IsListType.
// TODO: remove and replace with an interface check
isListCache = struct {
lock sync.RWMutex
byType map[reflect.Type]bool
}{
byType: make(map[reflect.Type]bool, 1024),
}
)
// IsListType returns true if the provided Object has a slice called Items.
// TODO: Replace the code in this check with an interface comparison by
// creating and enforcing that lists implement a list accessor.
func IsListType(obj runtime.Object) bool {
// if we're a runtime.Unstructured, check whether this is a list.
// TODO: refactor GetItemsPtr to use an interface that returns []runtime.Object
if unstructured, ok := obj.(runtime.Unstructured); ok {
return unstructured.IsList()
switch t := obj.(type) {
case runtime.Unstructured:
return t.IsList()
}
t := reflect.TypeOf(obj)
isListCache.lock.RLock()
ok, exists := isListCache.byType[t]
isListCache.lock.RUnlock()
if !exists {
_, err := getItemsPtr(obj)
ok = err == nil
// cache only the first 1024 types
isListCache.lock.Lock()
if len(isListCache.byType) < 1024 {
isListCache.byType[t] = ok
}
isListCache.lock.Unlock()
}
_, err := GetItemsPtr(obj)
return err == nil
return ok
}
var (
errExpectFieldItems = errors.New("no Items field in this object")
errExpectSliceItems = errors.New("Items field must be a slice of objects")
)
// GetItemsPtr returns a pointer to the list object's Items member.
// If 'list' doesn't have an Items member, it's not really a list type
// and an error will be returned.
// This function will either return a pointer to a slice, or an error, but not both.
// TODO: this will be replaced with an interface in the future
func GetItemsPtr(list runtime.Object) (interface{}, error) {
obj, err := getItemsPtr(list)
if err != nil {
return nil, fmt.Errorf("%T is not a list: %v", err)
}
return obj, nil
}
// getItemsPtr returns a pointer to the list object's Items member or an error.
func getItemsPtr(list runtime.Object) (interface{}, error) {
v, err := conversion.EnforcePtr(list)
if err != nil {
return nil, err
@ -48,19 +94,19 @@ func GetItemsPtr(list runtime.Object) (interface{}, error) {
items := v.FieldByName("Items")
if !items.IsValid() {
return nil, fmt.Errorf("no Items field in %#v", list)
return nil, errExpectFieldItems
}
switch items.Kind() {
case reflect.Interface, reflect.Ptr:
target := reflect.TypeOf(items.Interface()).Elem()
if target.Kind() != reflect.Slice {
return nil, fmt.Errorf("items: Expected slice, got %s", target.Kind())
return nil, errExpectSliceItems
}
return items.Interface(), nil
case reflect.Slice:
return items.Addr().Interface(), nil
default:
return nil, fmt.Errorf("items: Expected slice, got %s", items.Kind())
return nil, errExpectSliceItems
}
}

View File

@ -51,6 +51,8 @@ func (s unstructuredNegotiatedSerializer) SupportedMediaTypes() []runtime.Serial
return []runtime.SerializerInfo{
{
MediaType: "application/json",
MediaTypeType: "application",
MediaTypeSubType: "json",
EncodesAsText: true,
Serializer: json.NewSerializer(json.DefaultMetaFactory, s.creator, s.typer, false),
PrettySerializer: json.NewSerializer(json.DefaultMetaFactory, s.creator, s.typer, true),
@ -61,9 +63,11 @@ func (s unstructuredNegotiatedSerializer) SupportedMediaTypes() []runtime.Serial
},
},
{
MediaType: "application/yaml",
EncodesAsText: true,
Serializer: json.NewYAMLSerializer(json.DefaultMetaFactory, s.creator, s.typer),
MediaType: "application/yaml",
MediaTypeType: "application",
MediaTypeSubType: "yaml",
EncodesAsText: true,
Serializer: json.NewYAMLSerializer(json.DefaultMetaFactory, s.creator, s.typer),
},
}
}

View File

@ -91,6 +91,10 @@ type Framer interface {
type SerializerInfo struct {
// MediaType is the value that represents this serializer over the wire.
MediaType string
// MediaTypeType is the first part of the MediaType ("application" in "application/json").
MediaTypeType string
// MediaTypeSubType is the second part of the MediaType ("json" in "application/json").
MediaTypeSubType string
// EncodesAsText indicates this serializer can be encoded to UTF-8 safely.
EncodesAsText bool
// Serializer is the individual object serializer for this media type.

View File

@ -17,6 +17,9 @@ limitations under the License.
package serializer
import (
"mime"
"strings"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/runtime/serializer/json"
@ -120,6 +123,15 @@ func newCodecFactory(scheme *runtime.Scheme, serializers []serializerType) Codec
Serializer: d.Serializer,
PrettySerializer: d.PrettySerializer,
}
mediaType, _, err := mime.ParseMediaType(info.MediaType)
if err != nil {
panic(err)
}
parts := strings.SplitN(mediaType, "/", 2)
info.MediaTypeType = parts[0]
info.MediaTypeSubType = parts[1]
if d.StreamSerializer != nil {
info.StreamSerializer = &runtime.StreamSerializerInfo{
Serializer: d.StreamSerializer,

View File

@ -1616,6 +1616,41 @@ func TestGet(t *testing.T) {
}
}
func BenchmarkGet(b *testing.B) {
storage := map[string]rest.Storage{}
simpleStorage := SimpleRESTStorage{
item: genericapitesting.Simple{
Other: "foo",
},
}
selfLinker := &setTestSelfLinker{
expectedSet: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/id",
name: "id",
namespace: "default",
}
storage["simple"] = &simpleStorage
handler := handleLinker(storage, selfLinker)
server := httptest.NewServer(handler)
defer server.Close()
u := server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/id"
b.ResetTimer()
for i := 0; i < b.N; i++ {
resp, err := http.Get(u)
if err != nil {
b.Fatalf("unexpected error: %v", err)
}
if resp.StatusCode != http.StatusOK {
b.Fatalf("unexpected response: %#v", resp)
}
if _, err := io.Copy(ioutil.Discard, resp.Body); err != nil {
b.Fatalf("unable to read body")
}
}
b.StopTimer()
}
func TestGetCompression(t *testing.T) {
storage := map[string]rest.Storage{}
simpleStorage := SimpleRESTStorage{

View File

@ -43,13 +43,13 @@ func MediaTypesForSerializer(ns runtime.NegotiatedSerializer) (mediaTypes, strea
// NegotiateOutputMediaType negotiates the output structured media type and a serializer, or
// returns an error.
func NegotiateOutputMediaType(req *http.Request, ns runtime.NegotiatedSerializer, restrictions EndpointRestrictions) (MediaTypeOptions, runtime.SerializerInfo, error) {
mediaType, ok := NegotiateMediaTypeOptions(req.Header.Get("Accept"), AcceptedMediaTypesForEndpoint(ns), restrictions)
mediaType, ok := NegotiateMediaTypeOptions(req.Header.Get("Accept"), ns.SupportedMediaTypes(), restrictions)
if !ok {
supported, _ := MediaTypesForSerializer(ns)
return mediaType, runtime.SerializerInfo{}, NewNotAcceptableError(supported)
}
// TODO: move into resthandler
info := mediaType.Accepted.Serializer
info := mediaType.Accepted
if (mediaType.Pretty || isPrettyPrint(req)) && info.PrettySerializer != nil {
info.Serializer = info.PrettySerializer
}
@ -58,12 +58,12 @@ func NegotiateOutputMediaType(req *http.Request, ns runtime.NegotiatedSerializer
// NegotiateOutputMediaTypeStream returns a stream serializer for the given request.
func NegotiateOutputMediaTypeStream(req *http.Request, ns runtime.NegotiatedSerializer, restrictions EndpointRestrictions) (runtime.SerializerInfo, error) {
mediaType, ok := NegotiateMediaTypeOptions(req.Header.Get("Accept"), AcceptedMediaTypesForEndpoint(ns), restrictions)
if !ok || mediaType.Accepted.Serializer.StreamSerializer == nil {
mediaType, ok := NegotiateMediaTypeOptions(req.Header.Get("Accept"), ns.SupportedMediaTypes(), restrictions)
if !ok || mediaType.Accepted.StreamSerializer == nil {
_, supported := MediaTypesForSerializer(ns)
return runtime.SerializerInfo{}, NewNotAcceptableError(supported)
}
return mediaType.Accepted.Serializer, nil
return mediaType.Accepted, nil
}
// NegotiateInputSerializer returns the input serializer for the provided request.
@ -99,10 +99,13 @@ func NegotiateInputSerializerForMediaType(mediaType string, streaming bool, ns r
func isPrettyPrint(req *http.Request) bool {
// DEPRECATED: should be part of the content type
if req.URL != nil {
pp := req.URL.Query().Get("pretty")
if len(pp) > 0 {
pretty, _ := strconv.ParseBool(pp)
return pretty
// avoid an allocation caused by parsing the URL query
if strings.Contains(req.URL.RawQuery, "pretty") {
pp := req.URL.Query().Get("pretty")
if len(pp) > 0 {
pretty, _ := strconv.ParseBool(pp)
return pretty
}
}
}
userAgent := req.UserAgent()
@ -139,17 +142,6 @@ func (emptyEndpointRestrictions) AllowsConversion(schema.GroupVersionKind, strin
func (emptyEndpointRestrictions) AllowsServerVersion(string) bool { return false }
func (emptyEndpointRestrictions) AllowsStreamSchema(s string) bool { return s == "watch" }
// AcceptedMediaType contains information about a valid media type that the
// server can serialize.
type AcceptedMediaType struct {
// Type is the first part of the media type ("application")
Type string
// SubType is the second part of the media type ("json")
SubType string
// Serializer is the serialization info this object accepts
Serializer runtime.SerializerInfo
}
// MediaTypeOptions describes information for a given media type that may alter
// the server response
type MediaTypeOptions struct {
@ -176,13 +168,13 @@ type MediaTypeOptions struct {
Unrecognized []string
// the accepted media type from the client
Accepted *AcceptedMediaType
Accepted runtime.SerializerInfo
}
// acceptMediaTypeOptions returns an options object that matches the provided media type params. If
// it returns false, the provided options are not allowed and the media type must be skipped. These
// parameters are unversioned and may not be changed.
func acceptMediaTypeOptions(params map[string]string, accepts *AcceptedMediaType, endpoint EndpointRestrictions) (MediaTypeOptions, bool) {
func acceptMediaTypeOptions(params map[string]string, accepts *runtime.SerializerInfo, endpoint EndpointRestrictions) (MediaTypeOptions, bool) {
var options MediaTypeOptions
// extract all known parameters
@ -208,7 +200,7 @@ func acceptMediaTypeOptions(params map[string]string, accepts *AcceptedMediaType
// controls the streaming schema
case "stream":
if len(v) > 0 && (accepts.Serializer.StreamSerializer == nil || !endpoint.AllowsStreamSchema(v)) {
if len(v) > 0 && (accepts.StreamSerializer == nil || !endpoint.AllowsStreamSchema(v)) {
return MediaTypeOptions{}, false
}
options.Stream = v
@ -236,16 +228,16 @@ func acceptMediaTypeOptions(params map[string]string, accepts *AcceptedMediaType
}
}
if options.Convert != nil && !endpoint.AllowsConversion(*options.Convert, accepts.Type, accepts.SubType) {
if options.Convert != nil && !endpoint.AllowsConversion(*options.Convert, accepts.MediaTypeType, accepts.MediaTypeSubType) {
return MediaTypeOptions{}, false
}
options.Accepted = accepts
options.Accepted = *accepts
return options, true
}
type candidateMediaType struct {
accepted *AcceptedMediaType
accepted *runtime.SerializerInfo
clauses goautoneg.Accept
}
@ -253,10 +245,10 @@ type candidateMediaTypeSlice []candidateMediaType
// NegotiateMediaTypeOptions returns the most appropriate content type given the accept header and
// a list of alternatives along with the accepted media type parameters.
func NegotiateMediaTypeOptions(header string, accepted []AcceptedMediaType, endpoint EndpointRestrictions) (MediaTypeOptions, bool) {
func NegotiateMediaTypeOptions(header string, accepted []runtime.SerializerInfo, endpoint EndpointRestrictions) (MediaTypeOptions, bool) {
if len(header) == 0 && len(accepted) > 0 {
return MediaTypeOptions{
Accepted: &accepted[0],
Accepted: accepted[0],
}, true
}
@ -266,8 +258,8 @@ func NegotiateMediaTypeOptions(header string, accepted []AcceptedMediaType, endp
for i := range accepted {
accepts := &accepted[i]
switch {
case clause.Type == accepts.Type && clause.SubType == accepts.SubType,
clause.Type == accepts.Type && clause.SubType == "*",
case clause.Type == accepts.MediaTypeType && clause.SubType == accepts.MediaTypeSubType,
clause.Type == accepts.MediaTypeType && clause.SubType == "*",
clause.Type == "*" && clause.SubType == "*":
candidates = append(candidates, candidateMediaType{accepted: accepts, clauses: clause})
}
@ -282,22 +274,3 @@ func NegotiateMediaTypeOptions(header string, accepted []AcceptedMediaType, endp
return MediaTypeOptions{}, false
}
// AcceptedMediaTypesForEndpoint returns an array of structs that are used to efficiently check which
// allowed media types the server exposes.
func AcceptedMediaTypesForEndpoint(ns runtime.NegotiatedSerializer) []AcceptedMediaType {
var acceptedMediaTypes []AcceptedMediaType
for _, info := range ns.SupportedMediaTypes() {
segments := strings.SplitN(info.MediaType, "/", 2)
if len(segments) == 1 {
segments = append(segments, "*")
}
t := AcceptedMediaType{
Type: segments[0],
SubType: segments[1],
Serializer: info,
}
acceptedMediaTypes = append(acceptedMediaTypes, t)
}
return acceptedMediaTypes
}

View File

@ -17,8 +17,10 @@ limitations under the License.
package negotiation
import (
"mime"
"net/http"
"net/url"
"strings"
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -39,7 +41,23 @@ type fakeNegotiater struct {
func (n *fakeNegotiater) SupportedMediaTypes() []runtime.SerializerInfo {
var out []runtime.SerializerInfo
for _, s := range n.types {
info := runtime.SerializerInfo{Serializer: n.serializer, MediaType: s, EncodesAsText: true}
mediaType, _, err := mime.ParseMediaType(s)
if err != nil {
panic(err)
}
parts := strings.SplitN(mediaType, "/", 2)
if len(parts) == 1 {
// this is an error on the server side
parts = append(parts, "")
}
info := runtime.SerializerInfo{
Serializer: n.serializer,
MediaType: s,
MediaTypeType: parts[0],
MediaTypeSubType: parts[1],
EncodesAsText: true,
}
for _, t := range n.streamTypes {
if t == s {
info.StreamSerializer = &runtime.StreamSerializerInfo{

View File

@ -292,7 +292,12 @@ func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) err
}
// setObjectSelfLink sets the self link of an object as needed.
// TODO: remove the need for the namer LinkSetters by requiring objects implement either Object or List
// interfaces
func setObjectSelfLink(ctx context.Context, obj runtime.Object, req *http.Request, namer ScopeNamer) error {
// We only generate list links on objects that implement ListInterface - historically we duck typed this
// check via reflection, but as we move away from reflection we require that you not only carry Items but
// ListMeta into order to be identified as a list.
if !meta.IsListType(obj) {
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {

View File

@ -512,6 +512,11 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
//
// test/integration/auth_test.go is currently the most comprehensive status code test
for _, s := range a.group.Serializer.SupportedMediaTypes() {
if len(s.MediaTypeSubType) == 0 || len(s.MediaTypeType) == 0 {
return nil, fmt.Errorf("all serializers in the group Serializer must have MediaTypeType and MediaTypeSubType set: %s", s.MediaType)
}
}
mediaTypes, streamMediaTypes := negotiation.MediaTypesForSerializer(a.group.Serializer)
allMediaTypes := append(mediaTypes, streamMediaTypes...)
ws.Produces(allMediaTypes...)

View File

@ -20,6 +20,7 @@ import (
"bufio"
"net"
"net/http"
"net/url"
"regexp"
"strconv"
"strings"
@ -34,7 +35,7 @@ import (
"k8s.io/apiserver/pkg/features"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"github.com/emicklei/go-restful"
restful "github.com/emicklei/go-restful"
"github.com/prometheus/client_golang/prometheus"
)
@ -234,7 +235,7 @@ func RecordLongRunning(req *http.Request, requestInfo *request.RequestInfo, comp
// a request. verb must be uppercase to be backwards compatible with existing monitoring tooling.
func MonitorRequest(req *http.Request, verb, group, version, resource, subresource, scope, component, contentType string, httpCode, respSize int, elapsed time.Duration) {
reportedVerb := cleanVerb(verb, req)
dryRun := cleanDryRun(req.URL.Query()["dryRun"])
dryRun := cleanDryRun(req.URL)
client := cleanUserAgent(utilnet.GetHTTPClient(req))
elapsedMicroseconds := float64(elapsed / time.Microsecond)
elapsedSeconds := elapsed.Seconds()
@ -331,12 +332,19 @@ func cleanVerb(verb string, request *http.Request) string {
return reportedVerb
}
func cleanDryRun(dryRun []string) string {
func cleanDryRun(u *url.URL) string {
// avoid allocating when we don't see dryRun in the query
if !strings.Contains(u.RawQuery, "dryRun") {
return ""
}
dryRun := u.Query()["dryRun"]
if errs := validation.ValidateDryRun(nil, dryRun); len(errs) > 0 {
return "invalid"
}
// Since dryRun could be valid with any arbitrarily long length
// we have to dedup and sort the elements before joining them together
// TODO: this is a fairly large allocation for what it does, consider
// a sort and dedup in a single pass
return strings.Join(utilsets.NewString(dryRun...).List(), ",")
}
@ -346,7 +354,10 @@ func cleanUserAgent(ua string) string {
return "Browser"
}
// If an old "kubectl.exe" has passed us its full path, we discard the path portion.
ua = kubectlExeRegexp.ReplaceAllString(ua, "$1")
if kubectlExeRegexp.MatchString(ua) {
// avoid an allocation
ua = kubectlExeRegexp.ReplaceAllString(ua, "$1")
}
return ua
}

View File

@ -17,7 +17,9 @@ limitations under the License.
package storage
import (
"mime"
"reflect"
"strings"
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -60,7 +62,24 @@ type fakeNegotiater struct {
func (n *fakeNegotiater) SupportedMediaTypes() []runtime.SerializerInfo {
var out []runtime.SerializerInfo
for _, s := range n.types {
info := runtime.SerializerInfo{Serializer: n.serializer, MediaType: s, EncodesAsText: true}
mediaType, _, err := mime.ParseMediaType(s)
if err != nil {
panic(err)
}
parts := strings.SplitN(mediaType, "/", 2)
if len(parts) == 1 {
// this is an error on the server side
parts = append(parts, "")
}
info := runtime.SerializerInfo{
Serializer: n.serializer,
MediaType: s,
MediaTypeType: parts[0],
MediaTypeSubType: parts[1],
EncodesAsText: true,
}
for _, t := range n.streamTypes {
if t == s {
info.StreamSerializer = &runtime.StreamSerializerInfo{

View File

@ -43,6 +43,8 @@ func init() {
var watchJsonSerializerInfo = runtime.SerializerInfo{
MediaType: "application/json",
MediaTypeType: "application",
MediaTypeSubType: "json",
EncodesAsText: true,
Serializer: json.NewSerializer(json.DefaultMetaFactory, watchScheme, watchScheme, false),
PrettySerializer: json.NewSerializer(json.DefaultMetaFactory, watchScheme, watchScheme, true),
@ -77,6 +79,8 @@ func (s basicNegotiatedSerializer) SupportedMediaTypes() []runtime.SerializerInf
return []runtime.SerializerInfo{
{
MediaType: "application/json",
MediaTypeType: "application",
MediaTypeSubType: "json",
EncodesAsText: true,
Serializer: json.NewSerializer(json.DefaultMetaFactory, basicScheme, basicScheme, false),
PrettySerializer: json.NewSerializer(json.DefaultMetaFactory, basicScheme, basicScheme, true),