Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: make GC codec agnostic #5347

Closed
wants to merge 1 commit into from
Closed

Conversation

Stebalien
Copy link
Member

This ensures that GC will work regardless of the codec on the CIDs returned by the blockstore.

This ensures that GC will work regardless of the codec on the CIDs returned by
the blockstore.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien Stebalien requested a review from Kubuxu as a code owner August 6, 2018 23:44
@ghost ghost assigned Stebalien Aug 6, 2018
@ghost ghost added the status/in-progress In progress label Aug 6, 2018
@Stebalien Stebalien requested a review from kevina August 6, 2018 23:45
@@ -76,14 +76,16 @@ func GC(ctx context.Context, bs bstore.GCBlockstore, dstor dstore.Datastore, pn
errors := false
var removed uint64

gchashes := MultihashSetFromCids(gcs)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we can't just use a multihash set when generating the colored set as we might visit a node with a raw CID first and then not visit its children when we visit it using a non-raw CID. Basically, traversal cares about CIDs.

@kevina
Copy link
Contributor

kevina commented Aug 7, 2018

Note this p.r. depends on a codec agnostic blockstore. Or part of #5231.

@Stebalien
Copy link
Member Author

As @kevina pointed out, this also doesn't fix block rm. Closing as we can resurrect this if we need it later.

@Stebalien Stebalien closed this Aug 7, 2018
@ghost ghost removed the status/in-progress In progress label Aug 7, 2018
@hacdias hacdias deleted the feat/codec-agnostic-gc branch May 9, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants