juicedata / juicefs

JuiceFS is a distributed POSIX file system built on top of Redis and S3.
https://juicefs.com
Apache License 2.0
10.48k stars 919 forks source link

Deleting files in an 'open' state can lead to quota calculation errors. #5018

Closed writeword closed 1 month ago

writeword commented 1 month ago

What happened: When an open file is deleted, the Unlink() method update dir quota. When the file is closed, the doDeleteSustainedInode() method update dir quota again. Causes the directory information seen with "df -i" or "df -h" to be inaccurate, and quota restrictions are inaccurate.

Corresponding code:

func (m *baseMeta) Unlink(ctx Context, parent Ino, name string, skipCheckTrash ...bool) syscall.Errno {
    if parent == RootInode && name == TrashName || isTrash(parent) && ctx.Uid() != 0 {
        return syscall.EPERM
    }
    if m.conf.ReadOnly {
        return syscall.EROFS
    }

    defer m.timeit("Unlink", time.Now())
    parent = m.checkRoot(parent)
    var attr Attr
    err := m.en.doUnlink(ctx, parent, name, &attr, skipCheckTrash...)
    if err == 0 {
        var diffLength uint64
        if attr.Typ == TypeFile {
            diffLength = attr.Length
        }
        m.updateDirStat(ctx, parent, -int64(diffLength), -align4K(diffLength), -1)
        m.updateDirQuota(ctx, parent, -align4K(diffLength), -1)
    }
    return err
}

func (m *kvMeta) doDeleteSustainedInode(sid uint64, inode Ino) error {
    var attr Attr
    var newSpace int64
    err := m.txn(func(tx *kvTxn) error {
        newSpace = 0
        a := tx.get(m.inodeKey(inode))
        if a == nil {
            return nil
        }
        newSpace = -align4K(attr.Length)
        m.parseAttr(a, &attr)
        tx.set(m.delfileKey(inode, attr.Length), m.packInt64(time.Now().Unix()))
        tx.delete(m.inodeKey(inode))
        tx.delete(m.sustainedKey(sid, inode))
        return nil
    }, inode)
    if err == nil && newSpace < 0 {
        m.updateStats(newSpace, -1)
        m.tryDeleteFileData(inode, attr.Length, false)
        m.updateDirQuota(Background, attr.Parent, newSpace, -1)
    }
    return err
}
davies commented 1 month ago

Yes, we should not do that twice