mfs: clean cache on sync

There have been multiple reports of people complaining that MFS slows down to a crawl and triggers OOM events when adding many items to a folder.

One underlying issue is that the MFS Directory has a cache which can grown unbounded. All items added to a directory are cached here even though they are already written to the underlying Unixfs folder on mfs writes.

The cache serves Child() requests only. The purpose is the a Directory can then give updated information about its children without being "up to date" on its own parent: when a new write happens to a folder, the parent folder needs to be notified about the new unixfs node of their child. With this cache, this notification can be delayed until a Flush() is requested, one of the children triggers a bubbling notification, or the parent folder is traversed.

When the parent folder is traversed, it will write cached entries of its childen to disk, including that of the current folder. In order to write the current folder it has triggered a GetNode() on it, which in turn has written all the cached nodes  in the current folder to disk. This cascades. In principle, leafs do not have children and should not need to be rewritten to unixfs, but without caching the TestConcurrentWrites tests start failing. Caching provides a way for concurrent writes to access a single reference to a unixfs node, somehow making things work.

Apart from cosmetic changes, this commits allows resetting the cache when it has been synced to disk. Other approaches have been tested, like fully removing the cache, or resetting it when it reaches certain size. All were insatisfactory in that MFS breaks in one way or other (i.e. when setting maxCacheSize to 1). Setting size to ~50 allows tests to pass, but just because no tests are testing with 50+1 items.

This change does not break any tests, but after what I've seen, I can assume that concurrent writes during a Flush event probably result in broken expectations, and I wouldn't be surprised if there is a way to concatenate writes, lookups and flushes on references to multiple folders on the tree and trigger errors where things just work now.

The main problem remains: the current setup essentially keeps all mfs in memory. This commit could allevaite the fact that reading the directory object triggers as many unixfs.AddChild() as nodes in the directory. After this, AddChild() would be triggered only on nodes cached since the last time.

I don't find a reliable way of fixing MFS and I have discovered multiple gotchas into what was going to be a quick fix.

CHANGELOG.md

@@ -33,6 +33,8 @@ The following emojis are used to highlight certain changes:
 
 ### Fixed
 
+* `mfs`: directory cache is now cleared every time the directory node is read, somewhat limiting unbounded growth and time to sync it to the underlying unixfs.
+
 ### Security
 
 ## [v0.25.0]

mfs/dir.go

@@ -99,10 +99,11 @@ func (d *Directory) localUpdate(c child) (*dag.ProtoNode, error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 
-	err := d.updateChild(c)
+	err := d.unixfsDir.AddChild(d.ctx, c.Name, c.Node)
 	if err != nil {
 		return nil, err
 	}
+
 	// TODO: Clearly define how are we propagating changes to lower layers
 	// like UnixFS.
 
@@ -125,31 +126,10 @@ func (d *Directory) localUpdate(c child) (*dag.ProtoNode, error) {
 	// TODO: Why do we need a copy?
 }
 
-// Update child entry in the underlying UnixFS directory.
-func (d *Directory) updateChild(c child) error {
-	err := d.unixfsDir.AddChild(d.ctx, c.Name, c.Node)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func (d *Directory) Type() NodeType {
 	return TDir
 }
 
-// childNode returns a FSNode under this directory by the given name if it exists.
-// it does *not* check the cached dirs and files
-func (d *Directory) childNode(name string) (FSNode, error) {
-	nd, err := d.childFromDag(name)
-	if err != nil {
-		return nil, err
-	}
-
-	return d.cacheNode(name, nd)
-}
-
 // cacheNode caches a node into d.childDirs or d.files and returns the FSNode.
 func (d *Directory) cacheNode(name string, nd ipld.Node) (FSNode, error) {
 	switch nd := nd.(type) {
@@ -219,7 +199,12 @@ func (d *Directory) childUnsync(name string) (FSNode, error) {
 		return entry, nil
 	}
 
-	return d.childNode(name)
+	nd, err := d.childFromDag(name)
+	if err != nil {
+		return nil, err
+	}
+
+	return d.cacheNode(name, nd)
 }
 
 type NodeListing struct {
@@ -361,29 +346,24 @@ func (d *Directory) AddChild(name string, nd ipld.Node) error {
 		return err
 	}
 
-	err = d.unixfsDir.AddChild(d.ctx, name, nd)
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return d.unixfsDir.AddChild(d.ctx, name, nd)
 }
 
-func (d *Directory) sync() error {
+func (d *Directory) syncCache(clean bool) error {
 	for name, entry := range d.entriesCache {
 		nd, err := entry.GetNode()
 		if err != nil {
 			return err
 		}
 
-		err = d.updateChild(child{name, nd})
+		err = d.unixfsDir.AddChild(d.ctx, name, nd)
 		if err != nil {
 			return err
 		}
 	}
-
-	// TODO: Should we clean the cache here?
-
+	if clean {
+		d.entriesCache = make(map[string]FSNode)
+	}
 	return nil
 }
 
@@ -408,7 +388,7 @@ func (d *Directory) GetNode() (ipld.Node, error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 
-	err := d.sync()
+	err := d.syncCache(true)
 	if err != nil {
 		return nil, err
 	}