Merge pull request #46588 from wojtek-t/matches_single_without_selector

Automatic merge from submit-queue (batch tested with PRs 45327, 46217, 46377, 46428, 46588)

Matches single without selector

This is reducing cpu-usage of apiserver by ~5% in 5000-node clusters.

staging/src/k8s.io/apimachinery/pkg/fields/selector.go

@@ -40,6 +40,8 @@ type Selector interface {
 
 	// Transform returns a new copy of the selector after TransformFunc has been
 	// applied to the entire selector, or an error if fn returns an error.
+	// If for a given requirement both field and value are transformed to empty
+	// string, the requirement is skipped.
 	Transform(fn TransformFunc) (Selector, error)
 
 	// Requirements converts this interface to Requirements to expose
@@ -79,6 +81,9 @@ func (t *hasTerm) Transform(fn TransformFunc) (Selector, error) {
 	if err != nil {
 		return nil, err
 	}
+	if len(field) == 0 && len(value) == 0 {
+		return Everything(), nil
+	}
 	return &hasTerm{field, value}, nil
 }
 
@@ -115,6 +120,9 @@ func (t *notHasTerm) Transform(fn TransformFunc) (Selector, error) {
 	if err != nil {
 		return nil, err
 	}
+	if len(field) == 0 && len(value) == 0 {
+		return Everything(), nil
+	}
 	return &notHasTerm{field, value}, nil
 }
 
@@ -169,13 +177,15 @@ func (t andTerm) RequiresExactMatch(field string) (string, bool) {
 }
 
 func (t andTerm) Transform(fn TransformFunc) (Selector, error) {
-	next := make([]Selector, len([]Selector(t)))
-	for i, s := range []Selector(t) {
+	next := make([]Selector, 0, len([]Selector(t)))
+	for _, s := range []Selector(t) {
 		n, err := s.Transform(fn)
 		if err != nil {
 			return nil, err
 		}
-		next[i] = n
+		if !n.Empty() {
+			next = append(next, n)
+		}
 	}
 	return andTerm(next), nil
 }

staging/src/k8s.io/apimachinery/pkg/fields/selector_test.go

@@ -325,3 +325,73 @@ func TestRequiresExactMatch(t *testing.T) {
 		}
 	}
 }
+
+func TestTransform(t *testing.T) {
+	testCases := []struct {
+		name      string
+		selector  string
+		transform func(field, value string) (string, string, error)
+		result    string
+		isEmpty   bool
+	}{
+		{
+			name:      "empty selector",
+			selector:  "",
+			transform: func(field, value string) (string, string, error) { return field, value, nil },
+			result:    "",
+			isEmpty:   true,
+		},
+		{
+			name:      "no-op transform",
+			selector:  "a=b,c=d",
+			transform: func(field, value string) (string, string, error) { return field, value, nil },
+			result:    "a=b,c=d",
+			isEmpty:   false,
+		},
+		{
+			name:     "transform one field",
+			selector: "a=b,c=d",
+			transform: func(field, value string) (string, string, error) {
+				if field == "a" {
+					return "e", "f", nil
+				}
+				return field, value, nil
+			},
+			result:  "e=f,c=d",
+			isEmpty: false,
+		},
+		{
+			name:      "remove field to make empty",
+			selector:  "a=b",
+			transform: func(field, value string) (string, string, error) { return "", "", nil },
+			result:    "",
+			isEmpty:   true,
+		},
+		{
+			name:     "remove only one field",
+			selector: "a=b,c=d,e=f",
+			transform: func(field, value string) (string, string, error) {
+				if field == "c" {
+					return "", "", nil
+				}
+				return field, value, nil
+			},
+			result:  "a=b,e=f",
+			isEmpty: false,
+		},
+	}
+
+	for i, tc := range testCases {
+		result, err := ParseAndTransformSelector(tc.selector, tc.transform)
+		if err != nil {
+			t.Errorf("[%d] unexpected error during Transform: %v", i, err)
+		}
+		if result.Empty() != tc.isEmpty {
+			t.Errorf("[%d] expected empty: %t, got: %t", i, tc.isEmpty, result.Empty)
+		}
+		if result.String() != tc.result {
+			t.Errorf("[%d] unexpected result: %s", i, result.String())
+		}
+	}
+
+}

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go

@@ -1032,7 +1032,16 @@ func (e *Store) Watch(ctx genericapirequest.Context, options *metainternalversio
 func (e *Store) WatchPredicate(ctx genericapirequest.Context, p storage.SelectionPredicate, resourceVersion string) (watch.Interface, error) {
 	if name, ok := p.MatchesSingle(); ok {
 		if key, err := e.KeyFunc(ctx, name); err == nil {
-			w, err := e.Storage.Watch(ctx, key, resourceVersion, p)
+			// For performance reasons, we can optimize the further computations of
+			// selector, by removing then "matches-single" fields, because they are
+			// already satisfied by choosing appropriate key.
+			sp, err := p.RemoveMatchesSingleRequirements()
+			if err != nil {
+				glog.Warningf("Couldn't remove matches-single requirements: %v", err)
+				// Since we couldn't optimize selector, reset to the original one.
+				sp = p
+			}
+			w, err := e.Storage.Watch(ctx, key, resourceVersion, sp)
 			if err != nil {
 				return nil, err
 			}

staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go

@@ -65,16 +65,41 @@ func (s *SelectionPredicate) MatchesLabelsAndFields(l labels.Set, f fields.Set)
 	return matched
 }
 
+const matchesSingleField = "metadata.name"
+
+func removeMatchesSingleField(field, value string) (string, string, error) {
+	if field == matchesSingleField {
+		return "", "", nil
+	}
+	return field, value, nil
+}
+
 // MatchesSingle will return (name, true) if and only if s.Field matches on the object's
 // name.
 func (s *SelectionPredicate) MatchesSingle() (string, bool) {
-	// TODO: should be namespace.name
-	if name, ok := s.Field.RequiresExactMatch("metadata.name"); ok {
+	if name, ok := s.Field.RequiresExactMatch(matchesSingleField); ok {
 		return name, true
 	}
 	return "", false
 }
 
+func (s *SelectionPredicate) RemoveMatchesSingleRequirements() (SelectionPredicate, error) {
+	var fieldsSelector fields.Selector
+	if s.Field != nil {
+		var err error
+		fieldsSelector, err = s.Field.Transform(removeMatchesSingleField)
+		if err != nil {
+			return SelectionPredicate{}, err
+		}
+	}
+	return SelectionPredicate{
+		Label:       s.Label,
+		Field:       fieldsSelector,
+		GetAttrs:    s.GetAttrs,
+		IndexFields: s.IndexFields,
+	}, nil
+}
+
 // For any index defined by IndexFields, if a matcher can match only (a subset)
 // of objects that return <value> for a given index, a pair (<index name>, <value>)
 // wil be returned.