Faster memory client

changelog/v0.30.10/sharedrefclient.yaml

@@ -0,0 +1,6 @@
+changelog:
+  - type: NEW_FEATURE
+    resolvesIssue: false
+    issueLink: https://github.com/solo-io/gloo/issues/7166
+    description: > 
+      Add an extension of the InMemoryResourceClient, called SharedRefResourceClient that does not clone resources on reads/writes.

pkg/api/v1/clients/factory/resource_client_factory.go

@@ -110,6 +110,8 @@ func newResourceClient(ctx context.Context, factory ResourceClientFactory, param
 		return file.NewResourceClient(opts.RootDir, resourceType), nil
 	case *MemoryResourceClientFactory:
 		return memory.NewResourceClient(opts.Cache, resourceType), nil
+	case *SharedRefMemoryResourceClientFactory:
+		return memory.NewSharedRefResourceClient(opts.Cache, resourceType), nil
 	case *KubeConfigMapClientFactory:
 		if opts.Cache == nil {
 			return nil, errors.Errorf("invalid opts, configmap client requires a kube core cache")
@@ -193,6 +195,14 @@ func (f *MemoryResourceClientFactory) NewResourceClient(ctx context.Context, par
 	return newResourceClient(ctx, f, params)
 }
 
+type SharedRefMemoryResourceClientFactory struct {
+	Cache memory.InMemoryResourceCache
+}
+
+func (f *SharedRefMemoryResourceClientFactory) NewResourceClient(ctx context.Context, params NewResourceClientParams) (clients.ResourceClient, error) {
+	return newResourceClient(ctx, f, params)
+}
+
 type KubeConfigMapClientFactory struct {
 	Clientset kubernetes.Interface
 	Cache     cache.KubeCoreCache

pkg/api/v1/clients/memory/resource_client.go

@@ -105,6 +105,9 @@ type ResourceClient struct {
 	resourceType resources.Resource
 	cache        InMemoryResourceCache
 }
+type SharedRefResourceClient struct {
+	ResourceClient
+}
 
 func NewResourceClient(cache InMemoryResourceCache, resourceType resources.Resource) *ResourceClient {
 	return &ResourceClient{
@@ -113,6 +116,15 @@ func NewResourceClient(cache InMemoryResourceCache, resourceType resources.Resou
 	}
 }
 
+func NewSharedRefResourceClient(cache InMemoryResourceCache, resourceType resources.Resource) *SharedRefResourceClient {
+	return &SharedRefResourceClient{
+		ResourceClient{
+			cache:        cache,
+			resourceType: resourceType,
+		},
+	}
+}
+
 var _ clients.ResourceClient = &ResourceClient{}
 
 func (rc *ResourceClient) Kind() string {
@@ -141,7 +153,6 @@ func (rc *ResourceClient) Read(namespace, name string, opts clients.ReadOpts) (r
 	clone := resources.Clone(resource)
 	return clone, nil
 }
-
 func (rc *ResourceClient) Write(resource resources.Resource, opts clients.WriteOpts) (resources.Resource, error) {
 	opts = opts.WithDefaults()
 	if err := resources.Validate(resource); err != nil {
@@ -267,3 +278,66 @@ func newOrIncrementResourceVer(resourceVersion string) string {
 	}
 	return fmt.Sprintf("%v", curr+1)
 }
+
+func (rc *SharedRefResourceClient) Read(namespace, name string, opts clients.ReadOpts) (resources.Resource, error) {
+	if err := resources.ValidateName(name); err != nil {
+		return nil, errors.Wrapf(err, "validation error")
+	}
+	opts = opts.WithDefaults()
+	resource, ok := rc.cache.Get(rc.key(namespace, name))
+	if !ok {
+		return nil, errors.NewNotExistErr(namespace, name)
+	}
+
+	return resource, nil
+}
+
+func (rc *SharedRefResourceClient) List(namespace string, opts clients.ListOpts) (resources.ResourceList, error) {
+	opts = opts.WithDefaults()
+	cachedResources := rc.cache.List(rc.Prefix(namespace))
+	var resourceList resources.ResourceList
+	for _, resource := range cachedResources {
+		if labels.SelectorFromSet(opts.Selector).Matches(labels.Set(resource.GetMetadata().Labels)) {
+			resourceList = append(resourceList, resource)
+		}
+	}
+
+	sort.Stable(resourceList)
+
+	return resourceList, nil
+}
+
+func (rc *SharedRefResourceClient) Write(resource resources.Resource, opts clients.WriteOpts) (resources.Resource, error) {
+	opts = opts.WithDefaults()
+	if err := resources.Validate(resource); err != nil {
+		return nil, errors.Wrapf(err, "validation error")
+	}
+
+	key := rc.key(resource.GetMetadata().GetNamespace(), resource.GetMetadata().GetName())
+
+	original, err := rc.Read(
+		resource.GetMetadata().GetNamespace(),
+		resource.GetMetadata().GetName(),
+		clients.ReadOpts{},
+	)
+	if original != nil && err == nil {
+		if !opts.OverwriteExisting {
+			return nil, errors.NewExistErr(resource.GetMetadata())
+		}
+		if resource.GetMetadata().GetResourceVersion() != original.GetMetadata().GetResourceVersion() {
+			return nil, errors.NewResourceVersionErr(
+				resource.GetMetadata().GetNamespace(),
+				resource.GetMetadata().GetName(),
+				resource.GetMetadata().GetResourceVersion(),
+				original.GetMetadata().GetResourceVersion(),
+			)
+		}
+	}
+
+	// initialize or increment resource version
+	resource.GetMetadata().ResourceVersion = newOrIncrementResourceVer(resource.GetMetadata().GetResourceVersion())
+
+	rc.cache.Set(key, resource)
+
+	return resource, nil
+}

pkg/api/v1/clients/memory/resource_client_test.go

@@ -18,77 +18,124 @@ import (
 )
 
 var _ = Describe("Base", func() {
-	var (
-		client *ResourceClient
-	)
-	BeforeEach(func() {
-		client = NewResourceClient(NewInMemoryResourceCache(), &v1.MockResource{})
-	})
-	AfterEach(func() {
-	})
-	It("CRUDs resources", func() {
-		selector := map[string]string{
-			helpers.TestLabel: helpers.RandString(8),
-		}
-		generic.TestCrudClient("ns1", "ns2", client, clients.WatchOpts{
-			Selector:    selector,
-			Ctx:         context.TODO(),
-			RefreshRate: time.Minute,
+	Context("Standard in memory client", func() {
+		var (
+			client *ResourceClient
+		)
+		BeforeEach(func() {
+			client = NewResourceClient(NewInMemoryResourceCache(), &v1.MockResource{})
 		})
-	})
-	It("should not return pointer to internal object", func() {
-		obj := &v1.MockResource{
-			Metadata: &core.Metadata{
-				Namespace: "ns",
-				Name:      "n",
-			},
-			Data: "test",
-		}
-		client.Write(obj, clients.WriteOpts{})
-		ret, err := client.Read("ns", "n", clients.ReadOpts{})
-		Expect(err).NotTo(HaveOccurred())
-		Expect(ret).NotTo(BeIdenticalTo(obj))
+		AfterEach(func() {
+		})
+		It("CRUDs resources", func() {
+			selector := map[string]string{
+				helpers.TestLabel: helpers.RandString(8),
+			}
+			generic.TestCrudClient("ns1", "ns2", client, clients.WatchOpts{
+				Selector:    selector,
+				Ctx:         context.TODO(),
+				RefreshRate: time.Minute,
+			})
+		})
+		It("should not return pointer to internal object", func() {
+			obj := &v1.MockResource{
+				Metadata: &core.Metadata{
+					Namespace: "ns",
+					Name:      "n",
+				},
+				Data: "test",
+			}
+			client.Write(obj, clients.WriteOpts{})
+			ret, err := client.Read("ns", "n", clients.ReadOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(ret).NotTo(BeIdenticalTo(obj))
 
-		ret2, err := client.Read("ns", "n", clients.ReadOpts{})
-		Expect(err).NotTo(HaveOccurred())
-		Expect(ret).NotTo(BeIdenticalTo(ret2))
+			ret2, err := client.Read("ns", "n", clients.ReadOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(ret).NotTo(BeIdenticalTo(ret2))
 
-		listret, err := client.List("ns", clients.ListOpts{})
-		Expect(err).NotTo(HaveOccurred())
-		Expect(listret[0]).NotTo(BeIdenticalTo(obj))
+			listret, err := client.List("ns", clients.ListOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(listret[0]).NotTo(BeIdenticalTo(obj))
 
-		listret2, err := client.List("ns", clients.ListOpts{})
-		Expect(err).NotTo(HaveOccurred())
-		Expect(listret[0]).NotTo(BeIdenticalTo(listret2[0]))
-	})
+			listret2, err := client.List("ns", clients.ListOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(listret[0]).NotTo(BeIdenticalTo(listret2[0]))
+		})
 
-	Context("Benchmarks", func() {
-		Measure("it should perform list efficiently", func(b Benchmarker) {
-			const numobjs = 10000
+		Context("Benchmarks", func() {
+			Measure("it should perform list efficiently", func(b Benchmarker) {
+				const numobjs = 10000
 
-			for i := 0; i < numobjs; i++ {
-				obj := &v1.MockResource{
-					Metadata: &core.Metadata{
-						Namespace: "ns",
-						Name:      fmt.Sprintf("n-%v", numobjs-i),
-					},
-					Data: strings.Repeat("123", 1000) + fmt.Sprintf("test-%v", i),
+				for i := 0; i < numobjs; i++ {
+					obj := &v1.MockResource{
+						Metadata: &core.Metadata{
+							Namespace: "ns",
+							Name:      fmt.Sprintf("n-%v", numobjs-i),
+						},
+						Data: strings.Repeat("123", 1000) + fmt.Sprintf("test-%v", i),
+					}
+					client.Write(obj, clients.WriteOpts{})
 				}
-				client.Write(obj, clients.WriteOpts{})
+				l := clients.ListOpts{}
+				var output resources.ResourceList
+				var err error
+				runtime := b.Time("runtime", func() {
+					output, err = client.List("ns", l)
+				})
+				Expect(err).NotTo(HaveOccurred())
+				Expect(output).To(HaveLen(numobjs))
+				Expect(output[0].GetMetadata().Name).To(Equal("n-1"))
+
+				Expect(runtime.Seconds()).Should(BeNumerically("<", 0.5), "List() shouldn't take too long.")
+			}, 10)
+
+		})
+	})
+
+	Context("Shared ref memory client", func() {
+		var (
+			client *SharedRefResourceClient
+		)
+		BeforeEach(func() {
+			client = NewSharedRefResourceClient(NewInMemoryResourceCache(), &v1.MockResource{})
+		})
+		AfterEach(func() {
+		})
+		It("CRUDs resources", func() {
+			selector := map[string]string{
+				helpers.TestLabel: helpers.RandString(8),
 			}
-			l := clients.ListOpts{}
-			var output resources.ResourceList
-			var err error
-			runtime := b.Time("runtime", func() {
-				output, err = client.List("ns", l)
+			generic.TestCrudClient("ns1", "ns2", client, clients.WatchOpts{
+				Selector:    selector,
+				Ctx:         context.TODO(),
+				RefreshRate: time.Minute,
 			})
+		})
+		It("should return pointer to internal object", func() {
+			obj := &v1.MockResource{
+				Metadata: &core.Metadata{
+					Namespace: "ns",
+					Name:      "n",
+				},
+				Data: "test",
+			}
+			client.Write(obj, clients.WriteOpts{})
+			ret, err := client.Read("ns", "n", clients.ReadOpts{})
 			Expect(err).NotTo(HaveOccurred())
-			Expect(output).To(HaveLen(numobjs))
-			Expect(output[0].GetMetadata().Name).To(Equal("n-1"))
+			Expect(ret).To(BeIdenticalTo(obj))
 
-			Expect(runtime.Seconds()).Should(BeNumerically("<", 0.5), "List() shouldn't take too long.")
-		}, 10)
+			ret2, err := client.Read("ns", "n", clients.ReadOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(ret).To(BeIdenticalTo(ret2))
 
-	})
+			listret, err := client.List("ns", clients.ListOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(listret[0]).To(BeIdenticalTo(obj))
 
+			listret2, err := client.List("ns", clients.ListOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(listret[0]).To(BeIdenticalTo(listret2[0]))
+		})
+	})
 })

test/tests/generic/test_crud_client.go

@@ -44,12 +44,12 @@ func TestCrudClient(namespace1, namespace2 string, client ResourceClient, opts c
 	ExpectWithOffset(testOffset, err).NotTo(HaveOccurred())
 	ExpectWithOffset(testOffset, list).To(BeEmpty())
 
-	r1, err := client.Write(input, clients.WriteOpts{})
+	r1, err := client.Write(resources.Clone(input), clients.WriteOpts{})
 	ExpectWithOffset(testOffset, err).NotTo(HaveOccurred())
 	postWrite(callbacks, r1)
 
 	_, err = client.Write(input, clients.WriteOpts{})
-	ExpectWithOffset(testOffset, err).To(HaveOccurred())
+	ExpectWithOffset(testOffset, err).To(HaveOccurred(), "Allowed overwriting resource without OverwriteExisting")
 	ExpectWithOffset(testOffset, errors.IsExist(err)).To(BeTrue())
 
 	ExpectWithOffset(testOffset, r1).To(BeAssignableToTypeOf(&v1.MockResource{}))
@@ -62,7 +62,7 @@ func TestCrudClient(namespace1, namespace2 string, client ResourceClient, opts c
 	_, err = client.Write(input, clients.WriteOpts{
 		OverwriteExisting: true,
 	})
-	ExpectWithOffset(testOffset, err).To(HaveOccurred())
+	ExpectWithOffset(testOffset, err).To(HaveOccurred(), "Wrote resource without changed version")
 
 	resources.UpdateMetadata(input, func(meta *core.Metadata) {
 		meta.ResourceVersion = r1.GetMetadata().ResourceVersion
@@ -99,6 +99,7 @@ func TestCrudClient(namespace1, namespace2 string, client ResourceClient, opts c
 		},
 	}
 	r2, err := client.Write(input, clients.WriteOpts{})
+	r2 = resources.Clone(r2)
 	ExpectWithOffset(testOffset, err).NotTo(HaveOccurred())
 
 	// with labels
@@ -125,7 +126,7 @@ func TestCrudClient(namespace1, namespace2 string, client ResourceClient, opts c
 		meta.ResourceVersion = ""
 	})
 	_, err = client.Write(r2, clients.WriteOpts{OverwriteExisting: true})
-	ExpectWithOffset(testOffset, err).To(HaveOccurred())
+	ExpectWithOffset(testOffset, err).To(HaveOccurred(), "Did not get version error")
 
 	err = client.Delete(namespace1, "adsfw", clients.DeleteOpts{})
 	ExpectWithOffset(testOffset, err).To(HaveOccurred())