Merge pull request #10915 from filecoin-project/fix/upgrade-sector-selection

sealing: Improve upgrade sector selection

documentation/en/default-lotus-miner-config.toml

@@ -515,15 +515,7 @@
   # env var: LOTUS_SEALING_MINUPGRADESECTOREXPIRATION
   #MinUpgradeSectorExpiration = 0
 
-  # When set to a non-zero value, minimum number of epochs until sector expiration above which upgrade candidates will
+  # DEPRECATED: Target expiration is no longer used
-  # be selected based on lowest initial pledge.
-  # 
-  # Target sector expiration is calculated by looking at the input deal queue, sorting it by deal expiration, and
-  # selecting N deals from the queue up to sector size. The target expiration will be Nth deal end epoch, or in case
-  # where there weren't enough deals to fill a sector, DealMaxDuration (540 days = 1555200 epochs)
-  # 
-  # Setting this to a high value (for example to maximum deal duration - 1555200) will disable selection based on
-  # initial pledge - upgrade sectors will always be chosen based on longest expiration
   #
   # type: uint64
   # env var: LOTUS_SEALING_MINTARGETUPGRADESECTOREXPIRATION

node/config/doc_gen.go

@@ -1106,15 +1106,7 @@ required to have expiration of at least the soonest-ending deal`,
 			Name: "MinTargetUpgradeSectorExpiration",
 			Type: "uint64",
 
-			Comment: `When set to a non-zero value, minimum number of epochs until sector expiration above which upgrade candidates will
+			Comment: `DEPRECATED: Target expiration is no longer used`,
-be selected based on lowest initial pledge.
-
-Target sector expiration is calculated by looking at the input deal queue, sorting it by deal expiration, and
-selecting N deals from the queue up to sector size. The target expiration will be Nth deal end epoch, or in case
-where there weren't enough deals to fill a sector, DealMaxDuration (540 days = 1555200 epochs)
-
-Setting this to a high value (for example to maximum deal duration - 1555200) will disable selection based on
-initial pledge - upgrade sectors will always be chosen based on longest expiration`,
 		},
 		{
 			Name: "CommittedCapacitySectorLifetime",

node/config/types.go

@@ -352,15 +352,7 @@ type SealingConfig struct {
 	// required to have expiration of at least the soonest-ending deal
 	MinUpgradeSectorExpiration uint64
 
-	// When set to a non-zero value, minimum number of epochs until sector expiration above which upgrade candidates will
+	// DEPRECATED: Target expiration is no longer used
-	// be selected based on lowest initial pledge.
-	//
-	// Target sector expiration is calculated by looking at the input deal queue, sorting it by deal expiration, and
-	// selecting N deals from the queue up to sector size. The target expiration will be Nth deal end epoch, or in case
-	// where there weren't enough deals to fill a sector, DealMaxDuration (540 days = 1555200 epochs)
-	//
-	// Setting this to a high value (for example to maximum deal duration - 1555200) will disable selection based on
-	// initial pledge - upgrade sectors will always be chosen based on longest expiration
 	MinTargetUpgradeSectorExpiration uint64
 
 	// CommittedCapacitySectorLifetime is the duration a Committed Capacity (CC) sector will

node/modules/storageminer.go

@@ -992,7 +992,6 @@ func NewSetSealConfigFunc(r repo.LockedRepo) (dtypes.SetSealingConfigFunc, error
 				WaitDealsDelay:                  config.Duration(cfg.WaitDealsDelay),
 				MakeNewSectorForDeals:           cfg.MakeNewSectorForDeals,
 				MinUpgradeSectorExpiration:      cfg.MinUpgradeSectorExpiration,
-				MinTargetUpgradeSectorExpiration: cfg.MinTargetUpgradeSectorExpiration,
 				MakeCCSectorsAvailable:          cfg.MakeCCSectorsAvailable,
 				AlwaysKeepUnsealedCopy:          cfg.AlwaysKeepUnsealedCopy,
 				FinalizeEarly:                   cfg.FinalizeEarly,
@@ -1032,7 +1031,6 @@ func ToSealingConfig(dealmakingCfg config.DealmakingConfig, sealingCfg config.Se
 		MaxSealingSectorsForDeals:  sealingCfg.MaxSealingSectorsForDeals,
 		PreferNewSectorsForDeals:   sealingCfg.PreferNewSectorsForDeals,
 		MinUpgradeSectorExpiration: sealingCfg.MinUpgradeSectorExpiration,
-		MinTargetUpgradeSectorExpiration: sealingCfg.MinTargetUpgradeSectorExpiration,
 		MaxUpgradingSectors:        sealingCfg.MaxUpgradingSectors,
 
 		StartEpochSealingBuffer:         abi.ChainEpoch(dealmakingCfg.StartEpochSealingBuffer),

storage/pipeline/input.go

@@ -417,6 +417,13 @@ func (m *Sealing) addPendingPiece(ctx context.Context, size abi.UnpaddedPieceSiz
 		close(pp.doneCh)
 	}
 
+	log.Debugw("new pending piece", "dealId", deal.DealID,
+		"piece", deal.DealProposal.PieceCID,
+		"size", size,
+		"dealStart", deal.DealSchedule.StartEpoch,
+		"dealEnd", deal.DealSchedule.EndEpoch,
+		"termEnd", ct.claimTermEnd)
+
 	m.pendingPieces[proposalCID(deal)] = pp
 	go func() {
 		defer m.inputLk.Unlock()
@@ -694,7 +701,6 @@ func (m *Sealing) maybeUpgradeSector(ctx context.Context, sp abi.RegisteredSealP
 		return &pieceBounds[f-1]
 	}
 
-	targetExpirationEpoch := ts.Height() + abi.ChainEpoch(cfg.MinTargetUpgradeSectorExpiration)
 	minExpirationEpoch := ts.Height() + abi.ChainEpoch(cfg.MinUpgradeSectorExpiration)
 
 	var candidate abi.SectorID
@@ -732,33 +738,25 @@ func (m *Sealing) maybeUpgradeSector(ctx context.Context, sp abi.RegisteredSealP
 			continue
 		}
 
+		if pb.dealBytesInBound.Padded() == 0 {
+			log.Debugw("skipping available sector", "sector", s.Number, "reason", "no deals in expiration bounds", "expiration", expirationEpoch)
+			continue
+		}
+
 		// if the sector has less than one sector worth of candidate deals, and
 		// the best candidate has more candidate deals, this sector isn't better
-		if pb.dealBytesInBound.Padded() < abi.PaddedPieceSize(ssize) {
-			if bestDealBytes > pb.dealBytesInBound.Padded() {
-				continue
-			}
-		}
 
-		// if best is below target, we want larger expirations
+		lessThanSectorOfData := pb.dealBytesInBound.Padded() < abi.PaddedPieceSize(ssize)
-		// if best is above target, we want lower pledge, but only if still above target
+		moreDealsThanBest := pb.dealBytesInBound.Padded() > bestDealBytes
 
-		// todo: after nv17 "target expiration" doesn't really make that much sense
+		// we want lower pledge, but only if we have more than one sector worth of deals
-		//  (tho to be fair it doesn't make too much sense now either)
-		//  we probably want the lowest expiration that's still above the configured
-		//  minimum, and can fit most candidate deals
 
-		if bestExpiration < targetExpirationEpoch {
+		preferDueToDealSize := lessThanSectorOfData && moreDealsThanBest
-			if expirationEpoch > bestExpiration && slowChecks(s.Number) {
+		preferDueToPledge := pledge.LessThan(bestPledge) && !lessThanSectorOfData
-				bestExpiration = expirationEpoch
-				bestPledge = pledge
-				bestDealBytes = pb.dealBytesInBound.Padded()
-				candidate = s
-			}
-			continue
-		}
 
-		if expirationEpoch >= targetExpirationEpoch && pledge.LessThan(bestPledge) && slowChecks(s.Number) {
+		prefer := preferDueToDealSize || preferDueToPledge
+
+		if prefer && slowChecks(s.Number) {
 			bestExpiration = expirationEpoch
 			bestPledge = pledge
 			bestDealBytes = pb.dealBytesInBound.Padded()
@@ -767,12 +765,12 @@ func (m *Sealing) maybeUpgradeSector(ctx context.Context, sp abi.RegisteredSealP
 	}
 
 	if bestExpiration < minExpirationEpoch {
-		log.Infow("Not upgrading any sectors", "available", len(m.available), "pieces", len(m.pendingPieces), "bestExp", bestExpiration, "target", targetExpirationEpoch, "min", minExpirationEpoch, "candidate", candidate)
+		log.Infow("Not upgrading any sectors", "available", len(m.available), "pieces", len(m.pendingPieces), "bestExp", bestExpiration, "min", minExpirationEpoch, "candidate", candidate)
 		// didn't find a good sector / no sectors were available
 		return false, nil
 	}
 
-	log.Infow("Upgrading sector", "number", candidate.Number, "type", "deal", "proofType", sp, "expiration", bestExpiration, "pledge", types.FIL(bestPledge))
+	log.Infow("Upgrading sector", "number", candidate.Number, "type", "deal", "proofType", sp, "expiration", bestExpiration, "pledge", types.FIL(bestPledge), "pieces", len(m.pendingPieces), "dealBytesAtExp", bestDealBytes)
 	delete(m.available, candidate)
 	m.nextDealSector = &candidate.Number
 	return true, m.sectors.Send(uint64(candidate.Number), SectorStartCCUpdate{})

storage/pipeline/sealiface/config.go

@@ -22,8 +22,6 @@ type Config struct {
 
 	MinUpgradeSectorExpiration uint64
 
-	MinTargetUpgradeSectorExpiration uint64
-
 	MaxUpgradingSectors uint64
 
 	MakeNewSectorForDeals bool