From b62008dacaa581c4f84bbdc11b0c739907b1ad15 Mon Sep 17 00:00:00 2001
From: Evgenii Stratonikov <e.stratonikov@yadro.com>
Date: Tue, 14 Nov 2023 14:35:23 +0300
Subject: [PATCH] [#506] node: Invalidate `list` cache after container
 add/removal

`update` already has problems mentioned in its doc-comment and the code
itself is not straightforward. Invalidating cache altogether seems like
a better option because we don't construct cache output ourselves (thus, no
"impossible" results).

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
---
 cmd/frostfs-node/cache.go     | 46 ++---------------------------------
 cmd/frostfs-node/container.go |  4 +--
 2 files changed, 4 insertions(+), 46 deletions(-)

diff --git a/cmd/frostfs-node/cache.go b/cmd/frostfs-node/cache.go
index 6a5d5d18..ea5bdfda 100644
--- a/cmd/frostfs-node/cache.go
+++ b/cmd/frostfs-node/cache.go
@@ -308,51 +308,9 @@ func (s ttlContainerLister) List(id *user.ID) ([]cid.ID, error) {
 	return res, nil
 }
 
-// updates cached list of owner's containers: cnr is added if flag is true, otherwise it's removed.
-// Concurrent calls can lead to some races:
-//   - two parallel additions to missing owner's cache can lead to only one container to be cached
-//   - async cache value eviction can lead to idle addition
-//
-// All described race cases aren't critical since cache values expire anyway, we just try
-// to increase cache actuality w/o huge overhead on synchronization.
-func (s *ttlContainerLister) update(owner user.ID, cnr cid.ID, add bool) {
+func (s *ttlContainerLister) invalidate(owner user.ID) {
 	strOwner := owner.EncodeToString()
-
-	val, ok := s.inner.cache.Peek(strOwner)
-	if !ok {
-		// we could cache the single cnr but in this case we will disperse
-		// with the Sidechain a lot
-		return
-	}
-
-	if s.inner.ttl <= time.Since(val.t) {
-		return
-	}
-
-	item := val.v
-
-	item.mtx.Lock()
-	{
-		found := false
-
-		for i := range item.list {
-			if found = item.list[i].Equals(cnr); found {
-				if !add {
-					item.list = append(item.list[:i], item.list[i+1:]...)
-					// if list became empty we don't remove the value from the cache
-					// since empty list is a correct value, and we don't want to insta
-					// re-request it from the Sidechain
-				}
-
-				break
-			}
-		}
-
-		if add && !found {
-			item.list = append(item.list, cnr)
-		}
-	}
-	item.mtx.Unlock()
+	s.inner.remove(strOwner)
 }
 
 type cachedIRFetcher struct {
diff --git a/cmd/frostfs-node/container.go b/cmd/frostfs-node/container.go
index e9a7079f..c7360a1b 100644
--- a/cmd/frostfs-node/container.go
+++ b/cmd/frostfs-node/container.go
@@ -74,7 +74,7 @@ func configureEACLAndContainerSources(c *cfg, client *cntClient.Client, cnrSrc c
 			//  creation success are most commonly tracked by polling GET op.
 			cnr, err := cnrSrc.Get(ev.ID)
 			if err == nil {
-				cachedContainerLister.update(cnr.Value.Owner(), ev.ID, true)
+				cachedContainerLister.invalidate(cnr.Value.Owner())
 				cachedContainerStorage.containerCache.set(ev.ID, cnr, nil)
 			} else {
 				// unlike removal, we expect successful receive of the container
@@ -96,7 +96,7 @@ func configureEACLAndContainerSources(c *cfg, client *cntClient.Client, cnrSrc c
 			cachedContainerStorage.handleRemoval(ev.ID)
 			info, err := cachedContainerStorage.DeletionInfo(ev.ID)
 			if err == nil {
-				cachedContainerLister.update(info.Owner, ev.ID, false)
+				cachedContainerLister.invalidate(info.Owner)
 			}
 
 			c.log.Debug(logs.FrostFSNodeContainerRemovalEventsReceipt,