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

fix: pin by source CID #1634

Merged
merged 1 commit into from
Mar 16, 2022
Merged

fix: pin by source CID #1634

merged 1 commit into from
Mar 16, 2022

Conversation

alanshaw
Copy link
Contributor

We need to pin by source CID to be consistent with all other uploads.

...also a fix to ensure the pin metadata is filtered correctly.

refs #1490

Part 2 is to pin all v0 source CIDs.

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 912823a
Status: ✅  Deploy successful!
Preview URL: https://41d13a69.nft-storage-1at.pages.dev

View logs

Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM

@alanshaw alanshaw merged commit 4ab48ce into main Mar 16, 2022
@alanshaw alanshaw deleted the fix/pin-by-source-cid branch March 16, 2022 11:36
alanshaw pushed a commit that referenced this pull request Apr 5, 2022
TLDR; we pin to cluster using `source_cid`, but the pins cron job checks status by `content_cid` so it is not found. For most data this is not a problem but when v0 CIDs are pinned their status never gets updated to `Pinned`.

This is a long standing bug that was masked by a regression that got fixed recently here: #1634

In Web3.Storage this query is harder so we actually downgrade `content_cid` to v0 when found to be unpinned everywhere and re-check with cluster: https://github.com/web3-storage/web3.storage/blob/3231807f24190de3e5be810ab25ed9567fda41cb/packages/cron/src/jobs/pins.js#L73-L75

Fixes #1745
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