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 bugs in sectors extend --v1-sectors #6066

Merged
merged 2 commits into from
Jul 11, 2021

Conversation

chadwick2143
Copy link
Contributor

@chadwick2143 chadwick2143 commented Apr 20, 2021

Fix bugs described in #6027
Based on the work of #6033

@jennijuju
Copy link
Member

jennijuju commented May 10, 2021

this could address #6180 too

@jennijuju jennijuju added the P2 P2: Should be resolved label May 24, 2021
@@ -507,6 +517,10 @@ var sectorsExtendCmd = &cli.Command{

// Set the new expiration to 48 hours less than the theoretical maximum lifetime
newExp := ml - (miner3.WPoStProvingPeriod * 2) + si.Activation
if withinTolerance(si.Expiration, newExp) || si.Expiration >= newExp {
Copy link
Contributor

Choose a reason for hiding this comment

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

Both of these should be redundant thanks to the check in line 514, but no reason to omit.

@@ -435,6 +435,12 @@ var sectorsExtendCmd = &cli.Command{
Usage: "when extending v1 sectors, don't try to extend sectors by fewer than this number of epochs",
Required: false,
},
&cli.Int64Flag{
Name: "expiration-ignore",
Value: 120,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest a larger default than 2 hours, but users need to be specifying this.

cmd/lotus-storage-miner/sectors.go Outdated Show resolved Hide resolved
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I've made one necessary change and this is good to go.

@arajasek arajasek mentioned this pull request Jul 11, 2021
@arajasek arajasek enabled auto-merge July 11, 2021 17:02
@arajasek
Copy link
Contributor

this could address #6180 too

It doesn't address it, but the solution to 6180 will share similar logic to the v1 sector extension logic.

@arajasek arajasek merged commit e983d90 into filecoin-project:master Jul 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 P2: Should be resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants