Convert GetRequestablePieces to an iterator

diff --git a/internal/request-strategy/order.go b/internal/request-strategy/order.go
index 5aee49f46df811e05030b7182371011fd085f0af..4384ea2c9752044808eafa6dae5ccb715bcbcf7c 100644 (file)
--- a/internal/request-strategy/order.go
+++ b/internal/request-strategy/order.go
@@ -25,7 +25,7 @@ type (
 func pieceOrderLess(i, j *PieceRequestOrderItem) multiless.Computation {
        return multiless.New().Int(
                int(j.State.Priority), int(i.State.Priority),
-               // TODO: Should we match on complete here to prevent churn when availability changes?
+               // TODO: Should we match on complete here to prevent churn when availability changes? (Answer: Yes).
        ).Bool(
                j.State.Partial, i.State.Partial,
        ).Int(
@@ -42,17 +42,16 @@ func pieceOrderLess(i, j *PieceRequestOrderItem) multiless.Computation {
        })
 }
 
-// Returns true if the piece should be considered against the unverified bytes limit. This is
-// based on whether the callee intends to request from the piece. Pieces submitted to this
-// callback passed Piece.Request and so are ready for immediate download.
+// This did return true if the piece should be considered against the unverified bytes limit. But
+// that would cause thrashing on completion: The order should be stable. This is now a 3-tuple
+// iterator.
 type RequestPieceFunc func(ih metainfo.Hash, pieceIndex int, orderState PieceRequestOrderState) bool
 
 // Calls f with requestable pieces in order.
 func GetRequestablePieces(
        input Input, pro *PieceRequestOrder,
-       // Returns true if the piece should be considered against the unverified bytes limit. This is
-       // based on whether the callee intends to request from the piece. Pieces submitted to this
-       // callback passed Piece.Request and so are ready for immediate download.
+       // Pieces submitted to this callback passed Piece.Request and so are ready for immediate
+       // download.
        requestPiece RequestPieceFunc,
 ) {
        // Storage capacity left for this run, keyed by the storage capacity pointer on the storage
@@ -82,7 +81,7 @@ func GetRequestablePieces(
                        if !requestPiece(ih, item.Key.Index, item.State) {
                                // No blocks are being considered from this piece, so it won't result in unverified
                                // bytes.
-                               return true
+                               return false
                        }
                } else if !piece.CountUnverified() {
                        // The piece is pristine, and we're not considering it for requests.

diff --git a/requesting.go b/requesting.go
index 63becb0ad8ae3a9412c14e779157103384b1c334..3dd40c52f8327a5a19ccfe0dddc96756565b1801 100644 (file)
--- a/requesting.go
+++ b/requesting.go
@@ -223,10 +223,10 @@ func (p *PeerConn) getDesiredRequestState() (desired desiredRequestState) {
        t.getRequestablePieces(
                func(ih metainfo.Hash, pieceIndex int, pieceExtra requestStrategy.PieceRequestOrderState) bool {
                        if ih != *t.canonicalShortInfohash() {
-                               return false
+                               return true
                        }
                        if !p.peerHasPiece(pieceIndex) {
-                               return false
+                               return true
                        }
                        requestHeap.pieceStates[pieceIndex].Set(pieceExtra)
                        allowedFast := p.peerAllowedFast.Contains(pieceIndex)

diff --git a/webseed-requesting.go b/webseed-requesting.go
index 15de04384f1e0c2010a56bce1c599ef3adedf164..a4cc17d5d4ef8701190399b6fd06e689859924e1 100644 (file)
--- a/webseed-requesting.go
+++ b/webseed-requesting.go
@@ -340,13 +340,12 @@ func (cl *Client) iterPossibleWebseedRequests() iter.Seq2[webseedUniqueRequestKe
                                func(ih metainfo.Hash, pieceIndex int, orderState requestStrategy.PieceRequestOrderState) bool {
                                        t := cl.torrentsByShortHash[ih]
                                        if len(t.webSeeds) == 0 {
-                                               return false
+                                               return true
                                        }
                                        p := t.piece(pieceIndex)
                                        cleanOpt := p.firstCleanChunk()
                                        if !cleanOpt.Ok {
-                                               // Could almost return true here, as clearly something is going on with the piece.
-                                               return false
+                                               return true
                                        }
                                        // Pretty sure we want this and not the order state priority. That one is for
                                        // client piece request order and ignores other states like hashing, marking
@@ -357,7 +356,7 @@ func (cl *Client) iterPossibleWebseedRequests() iter.Seq2[webseedUniqueRequestKe
                                        for url, ws := range t.webSeeds {
                                                // Return value from this function (RequestPieceFunc) doesn't terminate
                                                // iteration, so propagate that to not handling the yield return value.
-                                               yield(
+                                               if !yield(
                                                        webseedUniqueRequestKey{
                                                                aprioriWebseedRequestKey{
                                                                        t:          t,
@@ -370,7 +369,9 @@ func (cl *Client) iterPossibleWebseedRequests() iter.Seq2[webseedUniqueRequestKe
                                                                priority: priority,
                                                                costKey:  ws.hostKey,
                                                        },
-                                               )
+                                               ) {
+                                                       return false
+                                               }
                                        }
                                        // Pieces iterated here are only to select webseed requests. There's no guarantee they're chosen.
                                        return false