-
-
Notifications
You must be signed in to change notification settings - Fork 12
Automatically pin entries #6
Comments
This is becoming more urgent as GC is being added to the next js-ipfs release: ipfs/js-ipfs#2396 |
Note: GC in the next release is not run automaticallly when repo is full or on a schedule. You have to explicitly call the command if you want it to run. |
I think I'm beginning to lean toward leaving pinning (and unpinning) to the application (or perhaps even in (1) Not all use cases will want to pin by default, thus we will need to pass the choice to the user/app. (2) I believe we would have to make changes elsewhere to pin "replicated" entries as this would only address pinning locally created entries. Being that this isn't all that urgent yet, I'll experiment with this and report back once my thoughts are better formulated. |
Cool @mistakia! Makes sense that you might not always want to pin. In your mind is this only when the DB is opened temporarily? I'm trying to figure out in which cases this would be desired. |
Thanks for the heads up @oed @alanshaw! @mistakia I agree that not all use-cases will want to pin by default and applications could certainly benefit from some flexibility. A simple approach would be to pin per store, so only a flag in the options during store creation is required and the code-changes limited to passing the flag from How does adding a |
I think adding a |
@shamb0t I think that would be a good approach as it would handle the most common scenario and allow for applications to handle the rest. I also agree with @oed that it should be @oed - in my use case its desirable for a user to temporarily open a remote db to "browse". I spent some time this weekend implementing pinning and gc and came across these thoughts: (1) I found it much easier to implement outside of orbit-db land (lots of passing the flag around). With the exception of issues related to (4), it was quite easy to add pinning with orbit-db as-is. (2) Pinning an entry will also recursively pin links by default (although it seems that it will ignore links to objects that it does not have?). In my use case, it is desirable to pin log entries and some of the links contained within but not all. A pin flag per store would work as I could just set the flag to false and handle selectively pinning the links within the application. (3) In my use case, a db could be opened temporarily to "browse" but then the user may decide to keep it, at which point non-pinned entries that are already replicated will need to be pinned. I'm still not sure what approach would be best but I'm leaning toward this being handled at the application layer. I wonder if a simple method to change the pin flag could be useful for some. (4) Obviously, the manifest will need to be pinned. And in the case of an IPFS access controller the access controller address (address getter not working, but exposed on the options object) and write array address need to be pinned (which is currently not exposed to the application layer). Based on my experience, I think a simple, default on, pin flag per store is the way to go. Overall, contrary to my initial thoughts on this, I think it would be best for orbit-db to have a simple approach to pinning and allow for maximum flexibility as it is application specific and highly dependent on how data is structured within IPFS. |
I've been this out with ipfs-log and am getting a big performance hit when enabling pinning:
with pinning enabled:
Seems pinning shouldn't be enabled by default and may be done separately in batches. What do you guys think? You try it out the benchmarks by changing the boolean here https://github.com/orbitdb/ipfs-log/blob/upgrade/ipfs-0.40.0-rc.0/benchmarks/benchmark-append.js#L21 and running |
Let's leave pinning disabled by default for now 👍 |
Currently entries are added with
ipfs.dag.put
. This doesn't automatically pin the entries that are added to ipfs. This could be problematic when the ipfs garbage collection gets activated. To solve this after eachdag.put
callipfs.pin.add(<cid>)
should also be called.The text was updated successfully, but these errors were encountered: