refactor: optimize argocd-application-controller redis usage (#5345)

* refactor: controller uses two level caching to reduce number of redis calls

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

cmd/argocd-application-controller/commands/argocd_application_controller.go

@@ -77,6 +77,7 @@ func NewCommand() *cobra.Command {
 
 			cache, err := cacheSrc()
 			errors.CheckError(err)
+			cache.Cache.SetClient(cacheutil.NewTwoLevelClient(cache.Cache.GetClient(), 10*time.Minute))
 
 			settingsMgr := settings.NewSettingsManager(ctx, kubeClient, namespace)
 			kubectl := kubeutil.NewKubectl()

pkg/apis/application/v1alpha1/types.go

@@ -985,6 +985,20 @@ type ApplicationTree struct {
 	Hosts []HostInfo `json:"hosts,omitempty" protobuf:"bytes,3,rep,name=hosts"`
 }
 
+// Normalize sorts application tree nodes and hosts. The persistent order allows to
+// effectively compare previously cached app tree and allows to unnecessary Redis requests.
+func (t *ApplicationTree) Normalize() {
+	sort.Slice(t.Nodes, func(i, j int) bool {
+		return t.Nodes[i].FullName() < t.Nodes[j].FullName()
+	})
+	sort.Slice(t.OrphanedNodes, func(i, j int) bool {
+		return t.OrphanedNodes[i].FullName() < t.OrphanedNodes[j].FullName()
+	})
+	sort.Slice(t.Hosts, func(i, j int) bool {
+		return t.Hosts[i].Name < t.Hosts[j].Name
+	})
+}
+
 type ApplicationSummary struct {
 	// ExternalURLs holds all external URLs of application child resources.
 	ExternalURLs []string `json:"externalURLs,omitempty" protobuf:"bytes,1,opt,name=externalURLs"`
@@ -1053,6 +1067,11 @@ type ResourceNode struct {
 	CreatedAt       *metav1.Time            `json:"createdAt,omitempty" protobuf:"bytes,8,opt,name=createdAt"`
 }
 
+// FullName returns node full name
+func (n *ResourceNode) FullName() string {
+	return fmt.Sprintf("%s/%s/%s/%s", n.Group, n.Kind, n.Namespace, n.Name)
+}
+
 func (n *ResourceNode) GroupKindVersion() schema.GroupVersionKind {
 	return schema.GroupVersionKind{
 		Group:   n.Group,
@@ -1100,6 +1119,11 @@ type ResourceDiff struct {
 	Modified           bool   `json:"modified,omitempty" protobuf:"bytes,12,opt,name=modified"`
 }
 
+// FullName returns full name of a node that was used for diffing
+func (r *ResourceDiff) FullName() string {
+	return fmt.Sprintf("%s/%s/%s/%s", r.Group, r.Kind, r.Namespace, r.Name)
+}
+
 // ConnectionStatus represents connection status
 type ConnectionStatus = string
 

ui/src/app/applications/components/application-pod-view/pod-view.tsx

@@ -303,7 +303,8 @@ export class PodView extends React.Component<PodViewProps> {
                     renderMenu: () => this.props.nodeMenu(rnode)
                 };
             }
-
+        });
+        (tree.nodes || []).forEach((rnode: ResourceTreeNode) => {
             if (rnode.kind !== 'Pod') {
                 return;
             }

util/cache/appstate/cache.go

@@ -3,6 +3,7 @@ package appstate
 import (
 	"context"
 	"fmt"
+	"sort"
 	"time"
 
 	"github.com/go-redis/redis/v8"
@@ -61,6 +62,9 @@ func (c *Cache) GetAppManagedResources(appName string, res *[]*appv1.ResourceDif
 }
 
 func (c *Cache) SetAppManagedResources(appName string, managedResources []*appv1.ResourceDiff) error {
+	sort.Slice(managedResources, func(i, j int) bool {
+		return managedResources[i].FullName() < managedResources[j].FullName()
+	})
 	return c.SetItem(appManagedResourcesKey(appName), managedResources, c.appStateCacheExpiration, managedResources == nil)
 }
 
@@ -82,6 +86,9 @@ func (c *Cache) OnAppResourcesTreeChanged(ctx context.Context, appName string, c
 }
 
 func (c *Cache) SetAppResourcesTree(appName string, resourcesTree *appv1.ApplicationTree) error {
+	if resourcesTree != nil {
+		resourcesTree.Normalize()
+	}
 	err := c.SetItem(appResourcesTreeKey(appName), resourcesTree, c.appStateCacheExpiration, resourcesTree == nil)
 	if err != nil {
 		return err

util/cache/cache.go

@@ -78,6 +78,14 @@ type Cache struct {
 	client CacheClient
 }
 
+func (c *Cache) GetClient() CacheClient {
+	return c.client
+}
+
+func (c *Cache) SetClient(client CacheClient) {
+	c.client = client
+}
+
 func (c *Cache) SetItem(key string, item interface{}, expiration time.Duration, delete bool) error {
 	key = fmt.Sprintf("%s|%s", key, common.CacheVersion)
 	if delete {

util/cache/inmemory.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/gob"
+	"fmt"
 	"time"
 
 	gocache "github.com/patrickmn/go-cache"
@@ -29,6 +30,25 @@ func (i *InMemoryCache) Set(item *Item) error {
 	return nil
 }
 
+// HasSame returns true if key with the same value already present in cache
+func (i *InMemoryCache) HasSame(key string, obj interface{}) (bool, error) {
+	var buf bytes.Buffer
+	err := gob.NewEncoder(&buf).Encode(obj)
+	if err != nil {
+		return false, err
+	}
+
+	bufIf, found := i.memCache.Get(key)
+	if !found {
+		return false, nil
+	}
+	existingBuf, ok := bufIf.(bytes.Buffer)
+	if !ok {
+		panic(fmt.Errorf("InMemoryCache has unexpected entry: %v", existingBuf))
+	}
+	return bytes.Equal(buf.Bytes(), existingBuf.Bytes()), nil
+}
+
 func (i *InMemoryCache) Get(key string, obj interface{}) error {
 	bufIf, found := i.memCache.Get(key)
 	if !found {

util/cache/twolevelclient.go

@@ -0,0 +1,68 @@
+package cache
+
+import (
+	"context"
+	"time"
+
+	log "github.com/sirupsen/logrus"
+)
+
+// NewTwoLevelClient creates cache client that proxies requests to given external cache and tries to minimize
+// number of requests to external client by storing cache entries in local in-memory cache.
+func NewTwoLevelClient(client CacheClient, inMemoryExpiration time.Duration) *twoLevelClient {
+	return &twoLevelClient{inMemoryCache: NewInMemoryCache(inMemoryExpiration), externalCache: client}
+}
+
+type twoLevelClient struct {
+	inMemoryCache *InMemoryCache
+	externalCache CacheClient
+}
+
+// Set stores the given value in both in-memory and external cache.
+// Skip storing the value in external cache if the same value already exists in memory to avoid requesting external cache.
+func (c *twoLevelClient) Set(item *Item) error {
+	has, err := c.inMemoryCache.HasSame(item.Key, item.Object)
+	if has {
+		return nil
+	}
+	if err != nil {
+		log.Warnf("Failed to check key '%s' in in-memory cache: %v", item.Key, err)
+	}
+	err = c.inMemoryCache.Set(item)
+	if err != nil {
+		log.Warnf("Failed to save key '%s' in in-memory cache: %v", item.Key, err)
+	}
+	return c.externalCache.Set(item)
+}
+
+// Get returns cache value from in-memory cache if it present. Otherwise loads it from external cache and persists
+// in memory to avoid future requests to external cache.
+func (c *twoLevelClient) Get(key string, obj interface{}) error {
+	err := c.inMemoryCache.Get(key, obj)
+	if err == nil {
+		return nil
+	}
+
+	err = c.externalCache.Get(key, obj)
+	if err == nil {
+		_ = c.inMemoryCache.Set(&Item{Key: key, Object: obj})
+	}
+	return err
+}
+
+// Delete deletes cache for given key in both in-memory and external cache.
+func (c *twoLevelClient) Delete(key string) error {
+	err := c.inMemoryCache.Delete(key)
+	if err != nil {
+		return err
+	}
+	return c.externalCache.Delete(key)
+}
+
+func (c *twoLevelClient) OnUpdated(ctx context.Context, key string, callback func() error) error {
+	return c.externalCache.OnUpdated(ctx, key, callback)
+}
+
+func (c *twoLevelClient) NotifyUpdated(key string) error {
+	return c.externalCache.NotifyUpdated(key)
+}