From c54d09d14e0a045745fe6e6b965c0632c96f437d Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Thu, 20 Oct 2016 10:51:19 +0200 Subject: [PATCH] Optimize computing fields in pod --- pkg/registry/core/endpoint/strategy.go | 37 +++++++++++++++------ pkg/registry/core/endpoint/strategy_test.go | 6 +--- pkg/registry/core/node/strategy.go | 11 +++++- pkg/registry/core/pod/strategy.go | 15 ++++----- 4 files changed, 43 insertions(+), 26 deletions(-) diff --git a/pkg/registry/core/endpoint/strategy.go b/pkg/registry/core/endpoint/strategy.go index 078fcf1151a..4b7f881c5ee 100644 --- a/pkg/registry/core/endpoint/strategy.go +++ b/pkg/registry/core/endpoint/strategy.go @@ -26,7 +26,7 @@ import ( "k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/registry/generic" "k8s.io/kubernetes/pkg/runtime" - apistorage "k8s.io/kubernetes/pkg/storage" + pkgstorage "k8s.io/kubernetes/pkg/storage" "k8s.io/kubernetes/pkg/util/validation/field" ) @@ -80,16 +80,31 @@ func (endpointsStrategy) AllowUnconditionalUpdate() bool { } // MatchEndpoints returns a generic matcher for a given label and field selector. -func MatchEndpoints(label labels.Selector, field fields.Selector) apistorage.SelectionPredicate { - return apistorage.SelectionPredicate{Label: label, Field: field, GetAttrs: EndpointsAttributes} +func MatchEndpoints(label labels.Selector, field fields.Selector) pkgstorage.SelectionPredicate { + return pkgstorage.SelectionPredicate{ + Label: label, + Field: field, + GetAttrs: func(obj runtime.Object) (labels.Set, fields.Set, error) { + endpoints, ok := obj.(*api.Endpoints) + if !ok { + return nil, nil, fmt.Errorf("invalid object type %#v", obj) + } + + // Compute fields only if field selectors is non-empty + // (otherwise those won't be used). + // Those are generally also not needed if label selector does + // not match labels, but additional computation of it is expensive. + var endpointsFields fields.Set + if !field.Empty() { + endpointsFields = EndpointsToSelectableFields(endpoints) + } + return endpoints.Labels, endpointsFields, nil + }, + } } -// EndpointsAttributes returns the attributes of an endpoint such that a -// SelectionPredicate can match appropriately. -func EndpointsAttributes(obj runtime.Object) (objLabels labels.Set, objFields fields.Set, err error) { - endpoints, ok := obj.(*api.Endpoints) - if !ok { - return nil, nil, fmt.Errorf("invalid object type %#v", obj) - } - return endpoints.Labels, generic.ObjectMetaFieldsSet(&endpoints.ObjectMeta, true), nil +// EndpointsToSelectableFields returns a field set that represents the object +// TODO: fields are not labels, and the validation rules for them do not apply. +func EndpointsToSelectableFields(endpoints *api.Endpoints) fields.Set { + return generic.ObjectMetaFieldsSet(&endpoints.ObjectMeta, true) } diff --git a/pkg/registry/core/endpoint/strategy_test.go b/pkg/registry/core/endpoint/strategy_test.go index 252e33e1e01..689f6f4367e 100644 --- a/pkg/registry/core/endpoint/strategy_test.go +++ b/pkg/registry/core/endpoint/strategy_test.go @@ -25,14 +25,10 @@ import ( ) func TestSelectableFieldLabelConversions(t *testing.T) { - _, fieldsSet, err := EndpointsAttributes(&api.Endpoints{}) - if err != nil { - t.Fatal(err) - } apitesting.TestSelectableFieldLabelConversionsOfKind(t, registered.GroupOrDie(api.GroupName).GroupVersion.String(), "Endpoints", - fieldsSet, + EndpointsToSelectableFields(&api.Endpoints{}), nil, ) } diff --git a/pkg/registry/core/node/strategy.go b/pkg/registry/core/node/strategy.go index 79061f60a4b..dba13180a43 100644 --- a/pkg/registry/core/node/strategy.go +++ b/pkg/registry/core/node/strategy.go @@ -154,7 +154,16 @@ func MatchNode(label labels.Selector, field fields.Selector) pkgstorage.Selectio if !ok { return nil, nil, fmt.Errorf("not a node") } - return labels.Set(nodeObj.ObjectMeta.Labels), NodeToSelectableFields(nodeObj), nil + + // Compute fields only if field selectors is non-empty + // (otherwise those won't be used). + // Those are generally also not needed if label selector does + // not match labels, but additional computation of it is expensive. + var nodeFields fields.Set + if !field.Empty() { + nodeFields = NodeToSelectableFields(nodeObj) + } + return labels.Set(nodeObj.ObjectMeta.Labels), nodeFields, nil }, IndexFields: []string{"metadata.name"}, } diff --git a/pkg/registry/core/pod/strategy.go b/pkg/registry/core/pod/strategy.go index 80130ce9c63..40bd5f37cde 100644 --- a/pkg/registry/core/pod/strategy.go +++ b/pkg/registry/core/pod/strategy.go @@ -166,18 +166,15 @@ func MatchPod(label labels.Selector, field fields.Selector) storage.SelectionPre return nil, nil, fmt.Errorf("not a pod") } - // podLabels is already sitting there ready to be used. - // podFields is not available directly and requires allocation of a map. - // Only bother if the fields might be useful to determining the match. - // One common case is for a replication controller to set up a watch - // based on labels alone; in that case we can avoid allocating the field map. - // This is especially important in the apiserver. - podLabels := labels.Set(pod.ObjectMeta.Labels) + // Compute fields only if field selectors is non-empty + // (otherwise those won't be used). + // Those are generally also not needed if label selector does + // not match labels, but additional computation of it is expensive. var podFields fields.Set - if !field.Empty() && label.Matches(podLabels) { + if !field.Empty() { podFields = PodToSelectableFields(pod) } - return podLabels, podFields, nil + return labels.Set(pod.ObjectMeta.Labels), podFields, nil }, IndexFields: []string{"spec.nodeName"}, }