File vs File descriptor

[schomatis]

The fileDescriptor structure seems to be doing too much work which should be moved to the File structure.

[schomatis]

The fileDescriptor seems to be basically a wrapper for the DagModifier but the most confusing part is that it keeps a reference to the File that created it (inode) accessing all of its attributes, making these two very coupled structures.

[schomatis]

Could we collapse both under File? (Moving the DagModifier from fileDescriptor.) It already seems like a file can only have one file descriptor operating on it. So instead of keeping a reference to the file descriptor we would only operate through File.

@Stebalien WDYT?

[schomatis]

It already seems like a file can only have one file descriptor operating on it.

Taking a closer look at the desclock it seems that its design is meant to allow either one R/W file descriptor open or many RO file descriptors open simultaneously.

go-mfs/file.go

Lines 75 to 79 in 166e30e

	switch flags {
	case OpenReadOnly:
	fi.desclock.RLock()
	case OpenWriteOnly, OpenReadWrite:
	fi.desclock.Lock()

[Stebalien]

I believe the idea is that a File represents the file itself and a file descriptor is a handle to an open file. Basically, having a File is like calling open("/path...", 0). From there, one can move/rename the file but one can't read/write it without opening it.

As you noted, it also allows for many readers. However, that can actually be implemented without ever exposing the underlying "file" object to the user (that can be an implementation detail).

[schomatis]

Yes, that makes sense, we can't collapse both into one, but I need to review the API boundaries between the two and better define who does what.

[schomatis]

review the API boundaries between the two and better define who does what.

One thing to review is the relationship between DagModifier in the file descriptor and the ipld.Node in the file (from which the first one is created) and clearly define which one reflects the current state of the file and during which moments during MFS operaions.

[nitishm]

review the API boundaries between the two and better define who does what.

One thing to review is the relationship between DagModifier in the file descriptor and the ipld.Node in the file (from which the first one is created) and clearly define which one reflects the current state of the file and during which moments during MFS operaions.

So the DagModifier has a dagserv which is a reference to the File(.inode).dagService. Yet in a most places we use the DagModifier while in some we use fd.inode.dagservice

go-mfs/fd.go

Line 135 in e5a375d

err = fi.inode.dagService.Add(context.TODO(), nd)

Why not just use the fd.mod.dagserv instead ? This is really confusing (too many dagservices!

[schomatis]

This is really confusing (too many dagservices!

I can definitely sympathize with that, in almost all circumstances they are all accessing the same DAG service (created in the setupNode and related methods) so you can just think of it as the DAG service. Why everyone has one? Depends on the case, I too would like to reduce a bit the number of references to it, but sometimes it's simpler to copy the reference than to access someone else's.

[aschmahmann]

This repository is no longer maintained and has been copied over to Boxo. In an effort to avoid noise and crippling in the Boxo repo from the weight of issues of the past, we are closing most issues and PRs in this repo. Please feel free to open a new issue in Boxo (and reference this issue) if resolving this issue is still critical for unblocking or improving your usecase.

You can learn more in the FAQs for the Boxo repo copying/consolidation effort.