✨client: client.MatchingFields support multiple indexes

[halfcrazy]

add support for multiple indexes when using client.MatchingFields

Fix #612

[k8s-ci-robot]

Hi @halfcrazy. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[halfcrazy]

/hold

After reading #1838 (review) seems we need an atomic operation here.

[halfcrazy]

/unhold

[alvaroaleman]

/ok-to-test

[halfcrazy]

/retest

[alvaroaleman]

Thanks!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, halfcrazy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [alvaroaleman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tareksha]

hi @halfcrazy can you give a short explanation about the expected performance of this implementation? how does it grow in relation to: 1- number of objects in store, 2- number of selectors. my local tests show somewhat linear growth by both..

[halfcrazy]

hi @halfcrazy can you give a short explanation about the expected performance of this implementation? how does it grow in relation to: 1- number of objects in store, 2- number of selectors. my local tests show somewhat linear growth by both..

In my local environment, there is an obvious step between 1 and 2 indexes and linear growth for more indexes.

In the long term, if upstream supports ByIndexes there would be a performance improve to eliminate the gap between 1 and more indexes.

cpu: Intel(R) Core(TM) i7-1065G7 CPU @ 1.30GHz
BenchmarkByIndexes1-4   	    1784	    763295 ns/op
BenchmarkByIndexes1-4   	    1692	    757617 ns/op
BenchmarkByIndexes1-4   	    1701	    617487 ns/op
BenchmarkByIndexes1-4   	    1837	    697619 ns/op
BenchmarkByIndexes1-4   	    1886	    655327 ns/op
BenchmarkByIndexes2-4   	     488	   2438093 ns/op
BenchmarkByIndexes2-4   	     440	   2744782 ns/op
BenchmarkByIndexes2-4   	     444	   2654877 ns/op
BenchmarkByIndexes2-4   	     451	   2733612 ns/op
BenchmarkByIndexes2-4   	     412	   2699114 ns/op
BenchmarkByIndexes3-4   	     373	   3046349 ns/op
BenchmarkByIndexes3-4   	     344	   3228925 ns/op
BenchmarkByIndexes3-4   	     319	   5556805 ns/op
BenchmarkByIndexes3-4   	      86	  15180030 ns/op
BenchmarkByIndexes3-4   	     289	   3911239 ns/op
BenchmarkByIndexes4-4   	     319	   3426624 ns/op
BenchmarkByIndexes4-4   	     348	   3536687 ns/op
BenchmarkByIndexes4-4   	     364	   3647029 ns/op
BenchmarkByIndexes4-4   	     289	   4686571 ns/op
BenchmarkByIndexes4-4   	     276	   4342592 ns/op
BenchmarkByIndexes5-4   	     309	   4256660 ns/op
BenchmarkByIndexes5-4   	     332	   3915095 ns/op
BenchmarkByIndexes5-4   	     307	   4555571 ns/op
BenchmarkByIndexes5-4   	     283	   3688207 ns/op
BenchmarkByIndexes5-4   	     297	   3642235 ns/op

package internal

import (
	"fmt"
	"strconv"
	"strings"
	"testing"

	corev1 "k8s.io/api/core/v1"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/apimachinery/pkg/fields"
	"k8s.io/client-go/tools/cache"
)

func benchmarkByIndexes(sel fields.Selector, b *testing.B) {
	requires := sel.Requirements()
	indexFunc := func(val string) func(obj interface{}) ([]string, error) {
		return func(obj interface{}) ([]string, error) {
			pod := obj.(*corev1.Pod)
			if strings.Contains(pod.Labels["foo"], val) {
				return []string{KeyToNamespacedKey("", val)}, nil
			}
			return nil, nil
		}
	}

	index := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{
		FieldIndexName("idx1"): indexFunc("1"),
		FieldIndexName("idx2"): indexFunc("2"),
		FieldIndexName("idx3"): indexFunc("3"),
		FieldIndexName("idx4"): indexFunc("4"),
		FieldIndexName("idx5"): indexFunc("5"),
	})

	for i := 0; i < 15000; i++ {
		err := index.Add(&corev1.Pod{ObjectMeta: metav1.ObjectMeta{
			Name: fmt.Sprintf("test-pod-%d", i), Labels: map[string]string{"foo": strconv.Itoa(i)}}})
		if err != nil {
			b.Fatal(err)
		}
	}

	for i := 0; i < b.N; i++ {
		objs, err := byIndexes(index, requires, "")
		if err != nil {
			b.Fatal(err)
		}
		if len(objs) == 0 {
			b.Fatal("expect to match objs, got 0")
		}
	}
}

func BenchmarkByIndexes1(b *testing.B) {
	sel := fields.ParseSelectorOrDie("idx1=1")
	benchmarkByIndexes(sel, b)
}

func BenchmarkByIndexes2(b *testing.B) {
	sel := fields.ParseSelectorOrDie("idx1=1,idx2=2")
	benchmarkByIndexes(sel, b)
}

func BenchmarkByIndexes3(b *testing.B) {
	sel := fields.ParseSelectorOrDie("idx1=1,idx2=2,idx3=3")
	benchmarkByIndexes(sel, b)
}

func BenchmarkByIndexes4(b *testing.B) {
	sel := fields.ParseSelectorOrDie("idx1=1,idx2=2,idx3=3,idx4=4")
	benchmarkByIndexes(sel, b)
}

func BenchmarkByIndexes5(b *testing.B) {
	sel := fields.ParseSelectorOrDie("idx1=1,idx2=2,idx3=3,idx4=4,idx5=5")
	benchmarkByIndexes(sel, b)
}

[tareksha]

@halfcrazy interesting so this indexing, and maybe k8s client-go indexes in general, does not really provide map-like lookup performance. it is a more of a convenience method.