Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mfs: clean cache on sync #751

Merged
merged 3 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ The following emojis are used to highlight certain changes:

### Fixed

* `mfs`: directory cache is now cleared on Flush(), liberating the memory used by the otherwise ever-growing cache. References to directories and sub-directories should be renewed after flushing.

### Security

## [v0.25.0]
Expand Down
56 changes: 20 additions & 36 deletions mfs/dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
hsanjuan marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}

// TODO: Clearly define how are we propagating changes to lower layers
// like UnixFS.

Expand All @@ -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)
}

hsanjuan marked this conversation as resolved.
Show resolved Hide resolved
// 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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -338,7 +323,7 @@ func (d *Directory) Unlink(name string) error {
}

func (d *Directory) Flush() error {
nd, err := d.GetNode()
nd, err := d.getNode(true)
if err != nil {
return err
}
Expand All @@ -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) cacheSync(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)
}
hsanjuan marked this conversation as resolved.
Show resolved Hide resolved
return nil
}

Expand All @@ -405,10 +385,14 @@ func (d *Directory) Path() string {
}

func (d *Directory) GetNode() (ipld.Node, error) {
return d.getNode(false)
}

func (d *Directory) getNode(cacheClean bool) (ipld.Node, error) {
d.lock.Lock()
defer d.lock.Unlock()

err := d.sync()
err := d.cacheSync(cacheClean)
if err != nil {
return nil, err
}
Expand Down
3 changes: 1 addition & 2 deletions mfs/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,9 @@ func (fi *File) setNodeData(data []byte) error {
}

fi.nodeLock.Lock()
defer fi.nodeLock.Unlock()
fi.node = nd
parent := fi.inode.parent
name := fi.inode.name

fi.nodeLock.Unlock()
return parent.updateChildEntry(child{name, fi.node})
}
11 changes: 1 addition & 10 deletions mfs/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,16 +170,7 @@
func (kr *Root) FlushMemFree(ctx context.Context) error {
dir := kr.GetDirectory()

if err := dir.Flush(); err != nil {
return err
}

dir.lock.Lock()
defer dir.lock.Unlock()

clear(dir.entriesCache)

return nil
return dir.Flush()

Check warning on line 173 in mfs/root.go

View check run for this annotation

Codecov / codecov/patch

mfs/root.go#L173

Added line #L173 was not covered by tests
}

// updateChildEntry implements the `parent` interface, and signals
Expand Down
Loading