Since created and modified date fields are file system level attributes, was hoping they could get added to the UnixFS file metadata fields.

We see that a date time field has been added to the JS version of this https://github.com/ipfs/js-ipfs-unixfs/blob/master/packages/ipfs-unixfs/src/unixfs.proto.js#L23 so wanted to open the discussion on what you thought about both created and modified date fields and adding it to the UnixFS specifications.

The go-merkledag gx-version used here,

differs from the one in go-ipfs, which is causing build problems when using gx-go link to work on this repo through the go-ipfs one.

This also raises the more general question: every time a package dependency shared between the two repos is updated, should it be immediately updated in the other one to avoid breaking the build?

(edit: well, not immediately, but I mean, is there an alternative to doing a local gx update just so gx link can work? given that this extracted repo is meant to be used primarily with go-ipfs, the gx link scenario seems pretty common.)

Background: ipfs/boxo#387.

(This is not completely thought out but a broad description of what we need to do. Further discussion and testing will probably be needed.)

The idea is to eliminate the interface and unify Shard and shardValue. Make it evident that values (links to entries in the directory) are stored at the leaf nodes with only one value per node. The Shard at the moment contains many values but that is just because it encodes the child nodes itself as a performance improvement to avoid requesting more nodes at the DAG layer. The performance should be maintained but it needs to be clarified, the shard/shard-value logic should be encapsulated in the loadChild (and related functions).

Concretely:

• Remove the child interface and the shardValue.
• Make the children node a slice of Shard.
• Extend Shard to save the value it was once stored in the shardValue, that is, the key/value pair (that may need to be renamed).
• Unify every switch logic that handled the Shard and shardValue separately.
• Modify the loadChild to also create Shards instead of shardValues.

This is a breaking change that goes in line with ipfs/go-mfs#86 to hide implementation details as much as possible: instead of having a Directory (representing the BasicDirectory implementation) and HAMTShard UnixFS types we would just expose the Directory type representing the Directory interface without qualifying which Go type is implementing it (as this is something that can change on the fly and the consumer should not depend on it).

The code change is trivial, assigning @aschmahmann basically to analyze feasibility (now or in future releases).

Now that we are including it in Sync (#33).

It would also be helpful to review the entire function itself.

Continuation and expansion of #94 (comment). Discussing in its own issue as I think this might be too long for a review thread.

The final advice there was to remove the timeout and block on the fetch. I'm not challenging the given advice but giving the full context to make sure we're aligned here since this can have a potentially negative UX effect.

Brief background on the unsharding algorithm:

1. The user modifies the HAMT directory (adds/removes an entry).
2. The size has potentially changed. We need to evaluate if it has fallen below the threshold to switch to a basic directory. We do this in the same add/remove operation; we do not defer it to a later stage like the extraction of the CID in GetNode.
3. We don't want to fetch all the entries so we don't compute the total/exact size and instead just try to fetch as little as possible to prove we are still above the threshold.
4. If some shards are unavailable in the network (high delay, missing/removed, etc.) we might not be able to retrieve enough shards to make sure we are above the threshold but we can't also claim we are below it.
5. Since the priority is to have a deterministic relation between directory size and CID in case of doubt we fail the operation, even if we were able to perform the original add/remove.
6. We can never really be sure when a shard is missing or actually has a long delay so we use a timeout to decide when to fail (for the reason explained above).

Here I'd say: the user has asked us to do an operation, we should keep trying until they cancel.

The user asked to do an add/remove operation but it may not be aware that it entails a potentially big fetch of shards well beyond the ones that were needed for the original operation.

• Following this advice we would then remove the timeout and depend entirely on the client when to stop the operation the same way it had to, even before the sharding feature, as an add/remove operation can always be blocking if we don't find the necessary shards to operate on.

• The advantage is the reduction of complexity as explained by Steven in the original comment.

• The potential disadvantage is depending on a user-defined timeout that was designed for specific add/remove operations and not for massive these "extra" fetches that might exceed the ones needed originally by a noticeable degree.

If we're comfortable going this way a simple 'go for it' is enough; no need to keep expanding on the motivation to remove the timeout.

Maybe we could use ResolveLink in node.go(https://github.com/ipfs/go-merkledag/blob/master/node.go#L359)

WDYT? @schomatis

unixfs constitutes a major interpretation of IPFS data into concrete interpretation.
It currently makes use of go-merkeldag, which exposes a primary dagservice interface.
In addition to this interface, elements of unixfs have made extensive use of the concrete protonode implementation used by merkeldag (example)

It would be valuable to have this code transitioned to make use of a nascent DagStore providing similar functionality on go-ipld-prime. This will help to allow for continued performant and clean use of unixfs within the context of ipld-prime backed data fetching.

• should FSNode continue to use the protobuf-generated struct implementation, or should we generate an equivalent ipld-prime node representation of that data?
• enumerate the interfaces (e.g. a quick glance shows public methods like NewShard make use of a dag service) that would ideally transition from reliance on ipld-format/go-merkeldag to prime equivalents (maybe you haves started this, @gammazero ?)
• Find consumers within the IPFS org that would need to update for this, make sure that's reasonable, and come up with a work plan (ordering of package updates / releases) that is mutually agreeable.

cc @aschmahmann @Stebalien @gammazero

See #30 (comment).

see ipfs/kubo#5217 (comment)

See #30 (comment).

The context used during read operations is always passed to the child PB node and "bifurcated" with a context.WithCancel(ctx) call,

I understand that the rationale behind this is that if I cancel a context at a certain depth in the DAG I'm reading, that is, if I cancel a fetch operation of a node (promise) in the DAG, it will cancel all the operations below that depth that are happening in their children.

That context being "bifurcated" always comes from the internal PBDagReader's context:

If we're doing a Read call (that calls CtxReadFull with the internal context) the behavior is consistent with the rationale described above, but if the user manually calls CtxReadFull with a custom context it will be applied only to the fetch operations (precalcNextBuf) of the root node but it won't be used for the child nodes at deeper depths,

The result is that a cancel of the custom context used in CtxReadFull won't cancel fetch operations (promises) at deeper levels, is that the intended behavior?

Inside io/directory.go, that should abstract the TDirectory/THAMTShard check.

Reference: https://github.com/ipfs/go-ipfs/milestone/38.

(I can't reference the milestone from the go-ipfs repo 😢 )

The importers wrap the DAG into an ipld.Batch which is hardcoded to do 128 MaxNodes because that's what works for flatfs.

I can see why in the context of go-ipfs optimizations, but it seems to me that at this point the importers should be agnostic to how the DAG service that they use is configured/optimized. Particularly it should be able to use a non-batching dag service, or one optimized differently.

My first impression is that they should probably use the DAGService as it comes it (without wrapping it) and it would be the user of the importer responsibility to call Commit when the importer has returned, if a Batch dag service was used in the first place.

I've realized this investigating some race conditions on cluster, where we use a custom DAGSerivce. It turns out that it becoming automatically batched to batches of 128 does not make any sense for us.

I would like to go ahead and "fix" this (here and in go-ipfs) if the intuitions above are correct...

I included this project as a module, but when I tried to build it I was served with the following errors;

# github.com/ipfs/go-merkledag
../../../../go/pkg/mod/github.com/ipfs/[email protected]/merkledag.go:83:16: format.ErrNotFound (type) is not an expression
../../../../go/pkg/mod/github.com/ipfs/[email protected]/merkledag.go:131:11: format.ErrNotFound (type) is not an expression
../../../../go/pkg/mod/github.com/ipfs/[email protected]/merkledag.go:148:15: format.ErrNotFound (type) is not an expression
../../../../go/pkg/mod/github.com/ipfs/[email protected]/merkledag.go:361:14: format.ErrNotFound (type) is not an expression
../../../../go/pkg/mod/github.com/ipfs/[email protected]/merkledag.go:374:14: format.ErrNotFound (type) is not an expression

This issue tracks small things that need to be done to wrap up the unsharding effort (concentrated in #94) but should be taken care of at the end as they are not very complex nor essential, and addressing them now might disrupt the current stack of PRs in development/review without adding much value.

Working in branch https://github.com/ipfs/go-unixfs/tree/schomatis/directory/unsharding-minor-issues.

• Document asynchronous choice (#94 (comment)).
• Change UpgradeableDirectory name (#94 (comment)).
• Un-export all Basic/HAMT switch functions (#94 (comment)).
• Make return values explicit in complex functions (#94 (comment), #94 (comment)).
• Rename inner HAMT functions to swap/take accordingly (#94 (comment), #94 (comment), #94 (comment)).
• Settle on a default number for HAMTShardingSize (#94 (comment)).
• Clean up getDagService() member function (#94 (comment)). (This may already happen as part of #99.)
• Reevaluate after reading the PR changing the threshold timeout calculations (#94 (comment)).

TLDR proposal:

• EITHER: add func (...) UnixPerms() ( os.FileMode ) { ... return os.FileMode( perms | os.ModeIrregular) }
• OR: extend the UnixFS mode spec to represent os.FileMode with full fidelity

Long version:

The exclusive under-specification of mode to only file permissions presents a problem when combined with the interface proposed by @Stebalien in ipfs/kubo#6920 (comment)

At present within the go-ipfs codebase the only way to derive the type of a UnixFS entry is to query the containing IPLD-ish object. E.g. https://github.com/ipfs/go-unixfs/blob/v0.2.4/file/unixfile.go#L153-L180, with the returned file.Node being differentiated based on the "derived-interface" ( Directory, File, etc ).

With introduction of an actual os.FileMode entry on such nodes, we are introducing an additional spot needing state synchronized. This is further exacerbated that the default of os.FileMode isn't "unknown" but rather "file".

It would be much clearer and less error-prone to introduce the new function into the interface as UnixPerms() or somesuch, returning the required 12 bit value ( which could be an os.FileMode object with ModeIrregular set ) . In the future if we do end up expanding the on-wire data to contain the actual filemode - we can keep UnixPerms() backwards compatible by making it a proxy over the new now-validated-always-correct FileMode().

Thoughts?

Background: ipfs/boxo#387.

The Shard object (UnixFS layer) stores the proto-node (DAG layer) from where it was created (NewHamtFromDag). It uses it as a kind of cache (mostly for its links I'm guessing) but its state is not clear at all (to the point that the Node method doesn't even update it) so internal functions can easily misuse it.

I'm not sure what are the ramifications of all this so the first step would be to remove it and replace it with a link slice copied from the original node (we were already copying the entire node in NewHamtFromDag so this won't take a performance hit). This slice would just be a lazy-loading mechanism of the Shards but once the Shard is loaded we would no longer have use for the link which could be niled (we need to check this). I really want to avoid the kind of double manipulation of the shard state like in insertChild where we update both children and nd.Links. From now on children would be the single place of truth to query the shard state. After this change we can evaluate actually having a cache in the form of a proto-node which actually would be updated in Node() to avoid re-marshaling everything every time the underlying DAG proto-node is needed (that is, if we modified a single entry in the HAMT directory only update the respective nodes but not the entire trie).

see comment #5326

The license section should make sure to conform to the IPFS licensing policy by linking the actual license file.

We should also make sure to link to the contributing guidelines in the contributing section.

Side notes:

• Is it correct that copyright is assigned to @jbenet here and not Protocol Labs, Inc?
• We don’t technically follow standard-readme here (e.g. no usage section, which is required). Should that note in the contributing section change?

refer to ipfs/kubo#5055

See ipfs/kubo#5316 (comment).

Good code example should stand on its own without relying to much on the rest of the IPFS codebase.

Hey @Stebalien,

I couldn't find any recent document/discussion about UnixFS v2. I was wondering how the design looks like, as I read some research about another filesystem that was created to have extremely fast access on folder levels, use hashes instead of btrees, and is designed to use very low code complexity.

Not sure if you guys heard if it yet it's SpadFS:

http://artax.karlin.mff.cuni.cz/~mikulas/spadfs/

While the structure on disk doesn't really make sense here, the layout of the directories might be interesting to have a look at.

Here are some graphs which compared it to other filesystems under Linux - especially the traverse a directory and add/remove files from directory performance is quite impressive.

https://www.semanticscholar.org/paper/Design-and-Implementation-of-the-Spad-Filesystem-Patocka/2fc025b8bdd14b7457802795df40fbbfd967a407