kahing / goofys

a high-performance, POSIX-ish Amazon S3 file system written in Go
Apache License 2.0
5.18k stars 522 forks source link

Leaking inodes when they're not looked up #395

Open ikonst opened 5 years ago

ikonst commented 5 years ago

I've been trying to diagnose numerous inode leaks in goofys. To do this, I added diagnostics to the SIGUSR1 handler:

var ids []int
for _, v := range fs.inodes {
    ids = append(ids, int(v.Id))
}
sort.Sort(sort.Reverse(sort.IntSlice(ids)))

for _, inodeId := range ids {
    inode := fs.inodes[fuseops.InodeID(inodeId)]
    if inode == nil || inode.refcnt != 0 {
        continue
    }
    var parentID fuseops.InodeID
    if inode.Parent != nil {
        parentID = inode.Parent.Id
    } else {
        parentID = 99999999
    }
    fmt.Printf("leaked: inode %d (%s) with parent %d\n", inode.Id, *inode.FullName(), parentID)
}

I created a simple bucket with a few files and directories.

I understand the reason for those inodes is the anticipation of a lookup, as per the comment:

// these are fake dir entries, we will realize the refcnt when
// lookup is done
inode.refcnt = 0

I'm not entirely sure about the right time to clean them. Is ReleaseDirHandle the right time? It's all made more complicated by the listObjects(Slurp) goroutine. Or should those inodes remain alive for a lookup that might come later? If so, for how long?

We seem to rely on the kernel looking them up, and then the kernel's own eviction policy (i.e. when it'll decide to send "ForgetInode"s) is what dictates their lifetime, but a "LookupInode" is not guaranteed.

kahing commented 5 years ago

sorry for the late response, but we should probably drop the children when we get a ForgetInode for a directory. Is this something you can try to submit a PR for?

ikonst commented 5 years ago

we should probably drop the children when we get a ForgetInode for a directory

I've never seen "ForgetInode"s for a directory. At best, there's "ReleaseDirHandle".

What was your original plan re: the lifetime of inodes? I had trouble understanding the "slurping" code, which objects it lists and what inodes it creates. Could you explain it on a sample hierarchy?