Call methods on Handle instead of Node when possible issue #92

[sjpotter]

An attempt at calling fsync/getattr/setattr on handlles when possible - this issue #92

I'm not thrilled with it, as a bit of code duplication (unsure how to do the if !nil and type assertion all together (which would eliminate the code duplication)

[sjpotter]

hmm, I see there's another PR that behaves similarly.

For me, the point was to be able to implement a passthrough FS. fsync requires me to pass an fd, which makes sense in a handle not in a Node.

now, I guess its conceptually possible for me to instead open / sync / close the file represented by the Node in the passthrough scenario, but that seems ugly.

[KyleSanderson]

@tv42 so, I'm implementing a r/w filesystem and need these calls (I can append to a file, but cannot truncate). nano is complaining about fsync, vi is all over the map, and du is returning 0.

Any update on landing this one to solve #92 ? I'm at the end here for a PoC but this is blocking me from continuing as I'm trying to not use any unsafe code (like shoving the raw FH* in the field). I'm new to golang but I'm happy to help try and move this forward.

[tv42]

On most filesystems, the file state belongs to the node not the handle; think of handle as just a view to the node. The handles for the same node should, normally, see the same data. In that world, handling fsync with operations on the node is the right thing to do, and will make your editor happy.

I'm not 100% against this, but it's extra API surface and decisions for the FS implementer (where do I handle sync), and it's moving away from the expected behavior of an FS.

[KyleSanderson]

so, how I've done this is FS is the parent implementation, Nodes are directories, Files are straight file entries on-disk (nodes as well), and finally FHs are the active file * (which means it's simply impossible to race).

fsync is implemented on the fd in C https://man7.org/linux/man-pages/man2/fsync.2.html, as well as ftruncate https://man7.org/linux/man-pages/man2/truncate.2.html, so it's a super weird one to me as this is the classic POSIX api.

Additionally https://libfuse.github.io/doxygen/structfuse__operations.html truncate and a bunch of other required calls seem to be missing so I'm not entirely certain if this library is the best fit for my use-case (replacing https://github.com/trapexit/mergerfs and friends in golang).

With no tuning, hitting the page cache I'm able to get GB/s on mediocre storage. There's issues filling the page cache (like reads are 10x slower than writes on first hit, which is bizarre but expected), there's trivial PRs like #238 that should resolve this situation but I haven't tried them yet.

Anyway, what do you think @tv42 - you're the incumbent here and I honestly have no idea beyond what I've written that passes what's expected from xfstests. 😄

[tv42]

You seem to be talking about C software. None of the code here has anything to do with libfuse. I will not even attempt to explain the decisions in libfuse.

For a passthrough FS, you should probably open the underlying fd in Lookup/Create/etc, and store it in the Node. Then all Handles can use the same fd. If you don't do that, you have no way of not having races and weird behavior on e.g. truncate(2); it's too late to access the backing file by name, you've already returned from Lookup.

[KyleSanderson]

You seem to be talking about C software.

Well, yes, that's the POSIX API outside of FUSE 😄. It's what everyone for the last 20+ years is accustomed to. While I understand this is golang (and what you've accomplished is something to be proud of), libfuse was/is Not Great®™©. However, this holds no ground here as we're talking about nearly straight syscalls that has nothing to do with FUSE.

you should probably open the underlying fd in Lookup/Create/etc, and store it in the Node.

I disagree on this one - this is guaranteed to blow things up. Each caller is guaranteed a handle, and then it moves to vfs for implementation details about how xfs wants to behave vs ext4 but this is nothing to do with FUSE.

If you don't do that, you have no way of not having races and weird behavior on e.g. truncate(2); it's too late to access the backing file by name, you've already returned from Lookup.

This races today in every ecosystem - my feeling is this all belongs on the filehandle as that's how this actually works underneath the hood (and why it's maintained relevance for nearly 40 years). Doing a read and/or write on a handle that's truncating is clownshoes - you're in for a horrible time if you cross the FILE * across boundaries. Multiple opens (os.open) however is certainly permitted across threads as this can happen anywhere outside the library.

Hopefully that helps explain what I'm seeing here. I don't expect this to be the be all end all, but I'm at-least hoping "write" support comes to bazil-fuse.

[tv42]

There's no FILE * anywhere in any picture that matters to me. Reads and writes and arbitrary-size truncations happening concurrently have well-defined behavior. I'm not sure how to communicate things better to you.

[KyleSanderson]

There's no FILE * anywhere in any picture that matters to me.

Got it. For me is the os.OpenFile * but I'll call this FILE * for sure.

Reads and writes and arbitrary-size truncations happening concurrently have well-defined behavior.

They do not. There's no way to do this reasonably with the present API. There's the setattr call but that's a hack that I can't use without this PR (I'm also unclear after chatting about it now that this actually fixes these editors from immediately erroring).

I'm not sure how to communicate things better to you.

We're still talking so I trust you understand I'm not trying to be malicious 😄. All I'm after is the best way to solve my problem which is mergerfs consumes atleast a full core on startup, and does not scale to the point where in a simple implementation I have 40 instances of the same application running. That's why I'm using this library, and trying to make the ecosystem better for all.

Hopefully that helps, if you have a IM/IRC/etc line or similar I'm happy to chat man - I just want to help.

EDIT:
Fixed the typo. Open is R/O in golang, OpenFile is the full featured implementation that I've been using since day 2.

[sjpotter]

just my 2c, the same logic that says these shouldn't be on the handle but on the node, means that reads/writes shouldn't be implemented on the handle either, but the node. i.e. why is their the HandleReader interface? all these should always be on the node.

[tv42]

@sjpotter A non-seekable file may be read at multiple offsets at once. Consider e.g. decompressing on the fly; the state for the decompressor goes in the Handle.

[sjpotter]

Not looking at the code right now, so this is mostly from memory (and therefore possibly wrong)

I agree that there is a good reason for read/write to be on handle, even though its actually impacting the node, but to a large extent that logic also applies to things like sync or any userspace api that is fd oriented (which the vast majority are). While they can be implemented on the node, that's extra bookkeeping that fs authors have to contend with (and get wrong). the fd (handle) is already opened, there's already a 1:1 mapping of fd the fuse fs is providing and the fd it has opened beneath it. for end users to either have to do extra fd bookeeping (ex: storing references to all open FDs in the handle, and randomly choosing one for these operations, or opening a new fd on demand) seems a waste when there's already a perfectly good fd ready.

The code I wrote isn't unique or even creative on my part, it was modeled after other areas already in bazil that have 2 interfaces for the same underlying functionality, on the node and handle and would chose one or the other based on which interface was defined on the underlying objects.

basically, if we're wrong to want this, we should be provided good example code demonstrating the better way to do this.

just my 2c.

[tv42]

I'm not saying you're wrong, I think I want this too, I'm just conservative with API additions to an already-complex API. And trying to point out when arguments presented didn't actually make sense.

This latest thing seems to be arguing from the point of view that the typical FUSE filesystem has a one-to-one Handle-to-backend-FD ratio, and I don't think that's true at all.

[sjpotter]

I'm not arguing that "typical" fuse filesystems are that way, but that legitimate fuse file systems can be that way.

i.e. any fuse file system that is a essentially a user space version of a stackable file system is likely to behave in such a way (at least in part)

https://www.linuxjournal.com/article/6485
https://www.filesystems.org/

i.e. a fuse filesystem that adds an encryption or compression layer on top of an existing file system, is likely 1:1 for many things.

[KyleSanderson]

@tv42

A non-seekable file may be read at multiple offsets at once. Consider e.g. decompressing on the fly; the state for the decompressor goes in the Handle.

Yes, totally. This would be a great optimization for a read/only workload as there's no need to maintain state like this. I additionally agree for a lot of FUSE filesystems writing is seen as something hard or complex and is typically not implemented. However, I do feel the need to stress you're coming at this from an optimisation perspective as opposed to in-the-field usage.

In my specific case however, I am indeed looking to write. So for the case of the archive you've mentioned, you could also open the compressed file on every single read, and if this overhead is high then migrate it to the parent node. However, one could very easily argue that this should be setup as a File Node that's filled from the parent Archive Node, which is nested in the Directory Node, which all belong to the FS implementation. Keeping the archive element node available keeps the archive open for every File implementer, which further reduces the overhead on this.

Hopefully that helps.

[KyleSanderson]

Apologies for cross-posting, the entire library actually seems to be incomplete / broken 😢 : bazil/zipfs#3

[tv42]

the entire library actually seems to be incomplete / broken

That attitude will get you just one pissed-off maintainer.

[sjpotter]

project seems dead, cleaning up my open PRs as no point to keep open.