-
Notifications
You must be signed in to change notification settings - Fork 48
Conversation
Pinned objects wouldn't be directly affected, but they wouldn't be protected from | ||
garbage collection anymore. | ||
|
||
We recently discovered a bug in the logic of our pinning code. Because of this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank @kyledrake explicitly, for finding and reporting this.
issue with the recursive hash trie implementation caused hash table buckets to | ||
be overwritten resulting in only 256 pins remaining in the pinset. After that, | ||
the bug won't be triggered again until the number of pins again exceeds 8192. | ||
The 256 pins that remain are random. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a paragraph on the fix. include:
- how we fixed it
- how we (stress) test that this no longer happens
- commitment to redouble efforts to make all our test suites more robust to ensure these kinds of problems do not happen.
- remind users not to run
ipfs repo gc
on sensitive data that is not otherwise backed up, as IPFS is still not 1.0 and our development may still find problems.
@lgierth and @whyrusleeping: can you review my changes? |
@RichardLitt I added a few more. @whyrusleeping @Kubuxu either of you have a last look too. Btw we need to fix building ipfs-see-all from source, which is mentioned as an option in the post. Rendered version: https://ipfs.io/ipfs/QmXarD5t3EDffn4nRtqVTcd6bJq5v5wFTErcLFxtg5MYk2/21-go-ipfs-0-4-4-released/ |
Ping @Kubuxu @whyrusleeping for a last fact-checking review. |
@Kubuxu @whyrusleeping @RichardLitt any updates? We should be able to ship this now. |
LGTM |
Great. Let's go! @lgierth publish? |
The "bug" and "find out if you're affected" sections still need a bit of wording polish.
cc @jbenet @whyrusleeping @RichardLitt