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

Util for miners to extend all v1 sectors #5924

Merged
merged 4 commits into from
Apr 5, 2021
Merged

Conversation

arajasek
Copy link
Contributor

@arajasek arajasek commented Mar 31, 2021

Run lotus-miner sectors extend --v1-sectors.

  • The tolerance flag can be passed to indicate what durations aren't "worth" extending. It defaults to one week, which means that sectors whose current lifetime's are within one week of the maximum possible lifetime will not be extended.

  • The expiration-cutoff flag can be passed to skip sectors whose expiration is past a certain point from the current head. It defaults to infinity (no cutoff), but if, say, 28800 was specified, then only sectors expiring in the next 10 days would be extended (2880 epochs in 1 day).

  • Tested manually on butterfly (kinda)

  • Should be tested on a large mainnet miner

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

No blocking changes (fine to approve as-is) but we could make this slightly nicer.

if cctx.Bool("v1-sectors") {
extensions := map[miner.SectorLocation]map[abi.ChainEpoch][]uint64{}
// are given durations within a week?
closeEnough := func(a, b abi.ChainEpoch) bool {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe make this an option?

Copy link
Member

Choose a reason for hiding this comment

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

Also, document.

}

for _, si := range sis {
if si.SealProof < abi.RegisteredSealProof_StackedDrg2KiBV1_1 {
Copy link
Member

Choose a reason for hiding this comment

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

maybe invert and use a continue?

for _, si := range sis {
if si.SealProof < abi.RegisteredSealProof_StackedDrg2KiBV1_1 {

ml := builtin3.SealProofPoliciesV11[si.SealProof].SectorMaxLifetime
Copy link
Member

Choose a reason for hiding this comment

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

Let's move this into chain/actors/policy and make it a function over the version.

cmd/lotus-storage-miner/sectors.go Outdated Show resolved Hide resolved
cmd/lotus-storage-miner/sectors.go Outdated Show resolved Hide resolved
}

newExp := ml - (miner3.WPoStProvingPeriod * 2) + si.Activation
p, err := api.StateSectorPartition(ctx, maddr, si.SectorNumber, types.EmptyTSK)
Copy link
Member

Choose a reason for hiding this comment

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

This is really inefficient (searches all partitions/deadlines). OK for a first pass, but this could be very slow for some miners.

On the other hand, may be not... let's wait and see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I tried it for the largest miners, and it is...not fast. Easy optimization is to only load the actor state once, but that is not gonna be the critical path. Alternative is to iterate over deadlines, and pre-compute one very large map of sectors -> locations, and then hit that up as needed.

This is a "needs to be run once" command, though, so eh?

for l, exts := range extensions {
for newExp, numbers := range exts {
scount += len(numbers)
if scount > miner.AddressedSectorsMax || len(p.Extensions) == miner3.DeclarationsMax {
Copy link
Member

Choose a reason for hiding this comment

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

nit: abstract over policy.

}
p := &miner3.ExtendSectorExpirationParams{}
Copy link
Member

Choose a reason for hiding this comment

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

why by reference? We're dereferencing everywhere.

}

p := &miner3.ExtendSectorExpirationParams{}
for l, numbers := range sectors {
Copy link
Member

Choose a reason for hiding this comment

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

Can we dedup this with the above loop? It's doing the exact same thing (except now we aren't sending multiple messages).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but eh.

Copy link
Member

Choose a reason for hiding this comment

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

Eh == we aren't sending multiple messages here so it'll fail in some cases (although I guess we can say that that's pretty unlikely).

@arajasek arajasek force-pushed the asr/extend-v1-util branch from ced47ca to 2855ec2 Compare April 4, 2021 02:04
@arajasek arajasek force-pushed the asr/extend-v1-util branch from 2855ec2 to 72c410c Compare April 4, 2021 02:19
@arajasek arajasek mentioned this pull request Apr 5, 2021
}

p := &miner3.ExtendSectorExpirationParams{}
for l, numbers := range sectors {
Copy link
Member

Choose a reason for hiding this comment

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

Eh == we aren't sending multiple messages here so it'll fail in some cases (although I guess we can say that that's pretty unlikely).

@arajasek arajasek merged commit f775737 into release/v1.6.0 Apr 5, 2021
@arajasek arajasek deleted the asr/extend-v1-util branch April 5, 2021 19:17
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.

3 participants