Page MenuHomeSoftware Heritage

FUSE: cache: add support to remove individual objects
ClosedPublic

Authored by haltode on Dec 28 2020, 3:13 PM.

Details

Summary

Invalidate specific cached artifact using rm cache/{SWHID}

Fix T2889.

Diff Detail

Repository
rDFUSE FUSE virtual file system
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D4795 (id=16971)

Rebasing onto 4b304d38ee...

Current branch diff-target is up to date.
Changes applied before test
commit 393876babe7b8bd14e5538e4c852d60c4f37f5fe
Author: Thibault Allançon <haltode@gmail.com>
Date:   Thu Dec 17 16:48:40 2020 +0100

    cache: add support to remove individual objects
    
    Invalidate specific cached artifact using `rm cache/{SWHID}`
    
    Fix T2889.

See https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/250/ for more details.

zack requested changes to this revision.Dec 28 2020, 4:09 PM
zack added a subscriber: zack.
zack added inline comments.
swh/fuse/fs/entry.py
63–64

do you really need to implement this here in this base class?

if it's just for documenting in the code that some sub-classes will override this, you can just raise NotImplementedError, as you do above

if this should really raise another error, than the error message here is incorrect, as it's not the entire filesystem that is read-only, but only most dirs in it

142

I know you asked me about this, but now that i rethink of it, permissions on symlinks usually don't matter, because they're never used for anything at all.
Did you have a reason to change this? If not, please revert this change.

swh/fuse/fuse.py
327–328

don't you need to also raise a FUSEError here, in case of exception? or am I missing the reason why it's not needed?

swh/fuse/tests/test_cache.py
21

s/invalidate/purge/

(as "cache invalidation" means a different thing)

33

This is a very surface-thing test, you're only testing that the directory entry is gone, not that the corresponding entry has actually been removed from the cache (which would be the actual underlying behavior).
Can you add that part, or is it too hidden/not reachable from the outside?

Unrelated question: is the FUSE cache emptied for sure before starting this test? (e.g., by test setup that creates a new instance) Because if it isn't you assertion here might fail if, for reasons unrelated to this test, there are other entries in the cache.

This revision now requires changes to proceed.Dec 28 2020, 4:09 PM
haltode added inline comments.
swh/fuse/fs/entry.py
142

I only changed the name to be consistent with RDONLY_FILE and RDONLY_DIR.

swh/fuse/tests/test_cache.py
33

I only have access to the path of the mounting dir in the unit tests, but since the cache/ listing is pretty much 1. read cache 2. create entries with sharded prefix, it should reflect directly the cache content.

For unit tests the cache is set to be created in-memory in the fuse_mntdir fixture, so yes it is empty everytime a test start.

Build is green

Patch application report for D4795 (id=16972)

Rebasing onto ae3a0e9760...

First, rewinding head to replay your work on top of it...
Applying: cache: add support to remove individual objects
Changes applied before test
commit 872f11b543e767a1cfa198975755474f3168ff8e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Thu Dec 17 16:48:40 2020 +0100

    cache: add support to remove individual objects
    
    Invalidate specific cached artifact using `rm cache/{SWHID}`
    
    Fix T2889.

See https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/251/ for more details.

zack added inline comments.
swh/fuse/fuse.py
329

Uhm, i'm not super-happy about always raising ENOENT in case of errors, as there might be other failure reasons, but looking around I see it's a common pattern also for other methods. So adding another instance won't make it worse.

This revision is now accepted and ready to land.Dec 28 2020, 7:49 PM

Build is green

Patch application report for D4795 (id=16974)

Rebasing onto ae3a0e9760...

Current branch diff-target is up to date.
Changes applied before test
commit bf87c60dd0003d9658ebe193a45f9bc0587e4887
Author: Thibault Allançon <haltode@gmail.com>
Date:   Thu Dec 17 16:48:40 2020 +0100

    cache: add support to remove individual objects
    
    Invalidate specific cached artifact using `rm cache/{SWHID}`
    
    Fix T2889.

See https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/252/ for more details.