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.

Sign up for GitHub

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

Jump to bottom

fix: cli: lotus-miner sectors extend command #11928

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: cli: lotus-miner sectors extend command #11928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 78 additions & 64 deletions cli/spcli/sectors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ func SectorsExtendCmd(getActorAddress ActorAddressGetter) *cli.Command {
&cli.Int64Flag{
Name: "max-sectors",
Usage: "the maximum number of sectors contained in each message",
Value: 500,
},
&cli.BoolFlag{
Name: "really-do-it",
Expand Down Expand Up @@ -876,84 +877,97 @@ func SectorsExtendCmd(getActorAddress ActorAddressGetter) *cli.Command {

for l, exts := range extensions {
for newExp, numbers := range exts {
sectorsWithoutClaimsToExtend := bitfield.New()
numbersToExtend := make([]abi.SectorNumber, 0, len(numbers))
var sectorsWithClaims []miner.SectorClaim
for _, sectorNumber := range numbers {
claimIdsToMaintain := make([]verifreg.ClaimId, 0)
claimIdsToDrop := make([]verifreg.ClaimId, 0)
cannotExtendSector := false
claimIds, ok := claimIdsBySector[sectorNumber]
// Nothing to check, add to ccSectors
if !ok {
sectorsWithoutClaimsToExtend.Set(uint64(sectorNumber))
numbersToExtend = append(numbersToExtend, sectorNumber)
} else {
for _, claimId := range claimIds {
claim, ok := claimsMap[claimId]
if !ok {
return xerrors.Errorf("failed to find claim for claimId %d", claimId)
}
claimExpiration := claim.TermStart + claim.TermMax
// can be maintained in the extended sector
if claimExpiration > newExp {
claimIdsToMaintain = append(claimIdsToMaintain, claimId)
} else {
sectorInfo, ok := activeSectorsInfo[sectorNumber]
batchSize := addrSectors

// The unfortunate thing about this approach is that batches less than batchSize in different partitions cannot be aggregated together to send messages.
for i := 0; i < len(numbers); i += batchSize {
end := i + batchSize
if end > len(numbers) {
end = len(numbers)
}

batch := numbers[i:end]

sectorsWithoutClaimsToExtend := bitfield.New()
numbersToExtend := make([]abi.SectorNumber, 0, len(numbers))
var sectorsWithClaims []miner.SectorClaim

for _, sectorNumber := range batch {
claimIdsToMaintain := make([]verifreg.ClaimId, 0)
claimIdsToDrop := make([]verifreg.ClaimId, 0)
cannotExtendSector := false
claimIds, ok := claimIdsBySector[sectorNumber]
// Nothing to check, add to ccSectors
if !ok {
sectorsWithoutClaimsToExtend.Set(uint64(sectorNumber))
numbersToExtend = append(numbersToExtend, sectorNumber)
} else {
for _, claimId := range claimIds {
claim, ok := claimsMap[claimId]
if !ok {
return xerrors.Errorf("failed to find sector in active sector set: %w", err)
return xerrors.Errorf("failed to find claim for claimId %d", claimId)
}
if !cctx.Bool("drop-claims") ||
// FIP-0045 requires the claim minimum duration to have passed
currEpoch <= (claim.TermStart+claim.TermMin) ||
// FIP-0045 requires the sector to be in its last 30 days of life
(currEpoch <= sectorInfo.Expiration-builtin.EndOfLifeClaimDropPeriod) {
fmt.Printf("skipping sector %d because claim %d (client f0%s, piece %s) does not live long enough \n", sectorNumber, claimId, claim.Client, claim.Data)
cannotExtendSector = true
break
claimExpiration := claim.TermStart + claim.TermMax
// can be maintained in the extended sector
if claimExpiration > newExp {
claimIdsToMaintain = append(claimIdsToMaintain, claimId)
} else {
sectorInfo, ok := activeSectorsInfo[sectorNumber]
if !ok {
return xerrors.Errorf("failed to find sector in active sector set: %w", err)
}
if !cctx.Bool("drop-claims") ||
// FIP-0045 requires the claim minimum duration to have passed
currEpoch <= (claim.TermStart+claim.TermMin) ||
// FIP-0045 requires the sector to be in its last 30 days of life
(currEpoch <= sectorInfo.Expiration-builtin.EndOfLifeClaimDropPeriod) {
fmt.Printf("skipping sector %d because claim %d (client f0%s, piece %s) does not live long enough \n", sectorNumber, claimId, claim.Client, claim.Data)
cannotExtendSector = true
break
}

claimIdsToDrop = append(claimIdsToDrop, claimId)
}

claimIdsToDrop = append(claimIdsToDrop, claimId)
numbersToExtend = append(numbersToExtend, sectorNumber)
}
if cannotExtendSector {
continue
}

numbersToExtend = append(numbersToExtend, sectorNumber)
}
if cannotExtendSector {
continue
if len(claimIdsToMaintain)+len(claimIdsToDrop) != 0 {
sectorsWithClaims = append(sectorsWithClaims, miner.SectorClaim{
SectorNumber: sectorNumber,
MaintainClaims: claimIdsToMaintain,
DropClaims: claimIdsToDrop,
})
}
}
}

if len(claimIdsToMaintain)+len(claimIdsToDrop) != 0 {
sectorsWithClaims = append(sectorsWithClaims, miner.SectorClaim{
SectorNumber: sectorNumber,
MaintainClaims: claimIdsToMaintain,
DropClaims: claimIdsToDrop,
})
}
sectorsWithoutClaimsCount, err := sectorsWithoutClaimsToExtend.Count()
if err != nil {
return xerrors.Errorf("failed to count cc sectors: %w", err)
}
}

sectorsWithoutClaimsCount, err := sectorsWithoutClaimsToExtend.Count()
if err != nil {
return xerrors.Errorf("failed to count cc sectors: %w", err)
}
sectorsInDecl := int(sectorsWithoutClaimsCount) + len(sectorsWithClaims)
scount += sectorsInDecl

sectorsInDecl := int(sectorsWithoutClaimsCount) + len(sectorsWithClaims)
scount += sectorsInDecl
if scount > addrSectors || len(p.Extensions) >= declMax {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a point for this check. If we are already batching and checking that batch size is smaller than policy max then what's the point of this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand what you mean, this part is the original logic, so I didn’t change it.
The first half of this judgment is redundant, and the second half of the inspection may still be useful. I had no one to communicate with at the time, so I chose not to adjust it in a conservative strategy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I delete the redundant first half?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I realized, there is nothing wrong here, it is completely wrong to remove the first half.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function here is to determine whether a new parameter should be opened. The function of the previous batching is to reduce the input batch size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's raining, 2300ml at a time
My barrel only has 500ml, and every time it rains 2300ml. My barrel is full, and there is still 1800ml (2300-500) left, and my other barrel cannot hold it.
So I divided the 2300ml each time into 500ml each time, so that my barrel can work normally (at this time, I also need to check whether the barrel is full)

params = append(params, p)
p = miner.ExtendSectorExpiration2Params{}
scount = sectorsInDecl
}

if scount > addrSectors || len(p.Extensions) >= declMax {
params = append(params, p)
p = miner.ExtendSectorExpiration2Params{}
scount = sectorsInDecl
p.Extensions = append(p.Extensions, miner.ExpirationExtension2{
Deadline: l.Deadline,
Partition: l.Partition,
Sectors: SectorNumsToBitfield(numbersToExtend),
SectorsWithClaims: sectorsWithClaims,
NewExpiration: newExp,
})
}

p.Extensions = append(p.Extensions, miner.ExpirationExtension2{
Deadline: l.Deadline,
Partition: l.Partition,
Sectors: SectorNumsToBitfield(numbersToExtend),
SectorsWithClaims: sectorsWithClaims,
NewExpiration: newExp,
})

}
}

Expand Down
2 changes: 1 addition & 1 deletion documentation/en/cli-lotus-miner.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ OPTIONS:
--drop-claims drop claims for sectors that can be extended, but only by dropping some of their verified power claims (default: false)
--tolerance value don't try to extend sectors by fewer than this number of epochs, defaults to 7 days (default: 20160)
--max-fee value use up to this amount of FIL for one message. pass this flag to avoid message congestion. (default: "0")
--max-sectors value the maximum number of sectors contained in each message (default: 0)
--max-sectors value the maximum number of sectors contained in each message (default: 500)
--really-do-it pass this flag to really extend sectors, otherwise will only print out json representation of parameters (default: false)
--help, -h show help
```
Expand Down
2 changes: 1 addition & 1 deletion documentation/en/cli-sptool.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ OPTIONS:
--drop-claims drop claims for sectors that can be extended, but only by dropping some of their verified power claims (default: false)
--tolerance value don't try to extend sectors by fewer than this number of epochs, defaults to 7 days (default: 20160)
--max-fee value use up to this amount of FIL for one message. pass this flag to avoid message congestion. (default: "0")
--max-sectors value the maximum number of sectors contained in each message (default: 0)
--max-sectors value the maximum number of sectors contained in each message (default: 500)
--really-do-it pass this flag to really extend sectors, otherwise will only print out json representation of parameters (default: false)
--help, -h show help
```
Expand Down