-
Notifications
You must be signed in to change notification settings - Fork 42
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
11 to 12: Improvements to migration #145
Conversation
a503a9e
to
844ea5a
Compare
This is already rebased on master |
@@ -7,6 +7,7 @@ require ( | |||
github.com/ipfs/fs-repo-migrations/tools v0.0.0-20211209222258-754a2dcb82ea | |||
github.com/ipfs/go-cid v0.0.7 | |||
github.com/ipfs/go-datastore v0.4.5 | |||
github.com/ipfs/go-ds-badger v0.2.7-0.20211210151007-a2805355dcf5 // indirect |
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.
The reliance on a non-tagged commit shouldn't hurt us here since we're using vendoring, right?
// this only works with a single worker. Otherwise we'd need | ||
// complex syncing, or delayed removal (increased datastore size). | ||
return cswap.runWorkers(1, swapWorkerFunc) | ||
return cswap.runWorkers(NWorkers, swapWorkerFunc) |
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.
From @hsanjuan: The logic for why we only needed 1 worker is not true anymore. Before we kept track of whether a reverted block had a v0 reference so we did not delete the v0... but now we don't delete anything on revert.
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.
LGTM
Fixes #144