search

holepunchto / hyperdrive

Hyperdrive is a secure, real time distributed file system
Apache License 2.0
1.86k stars 134 forks source link

Allow to add metadata to Stat entries #234

Closed Frando closed 5 years ago

Frando commented 5 years ago

This allows to add metadata to Stat entries. See the test for an example. (This PR is against the v10 branch)

mafintosh commented 5 years ago

@andrewosh @pfrazee this looked good to everyone right?

pfrazee commented 5 years ago

@mafintosh Yeah the mechanics look good. This is going to be a very cool feature.

I'm a little concerned about what happens if lots of data is written into the metadata, which might cause stat()s to incur larger performance costs than expected. (Think: people storing icons.) What do you think about adding a byte limit?

mafintosh commented 5 years ago

We def should add an advisory limit. We can add that in another PR though unless someone wants to add that quickly before I merge :)

Frando commented 5 years ago

Now that the hidden value support was added to hypertrie in hypertrie:#14 this PR should likely be reworked to use that api. The surface can either stay as it is so far (settings opts.metadata) or expose a new function instead (getMetadata, setMetadata). I propose to still keep the Map<name, bytes> schema for the metadata record so that different tools can use metadata without getting into conflicts.

Also, if I see things correctly, currently in hypertrie the hidden and non-hidden entries would have the same valueEncoding, right? If so then I'd propose to add support to hypertrie to set different valueEncoding for hidden and non-hidden nodes (so to have the regular hyperdrive Stat message for non-hidden nodes, and a Metadata message (with the name/bytes map) for the hidden nodes.

Opinions?

mafintosh commented 5 years ago

@Frando Yes, we need to make valueEncoding configurable for each command. I do like the simplicity of storing this in the stat object though, but if you were to store a ton of metadata the hidden trie would be def be better

Frando commented 5 years ago

I also thought about this some more and think that adding this is a good idea even with the hidden entries. Are there any TODOs left? For me it looks good.

Frando commented 5 years ago

Can we get this in before the v10 release?