From 697bb94f8f6bc37aca349d146dec057598397d98 Mon Sep 17 00:00:00 2001 From: zzz Date: Sat, 22 Aug 2026 10:22:50 -0400 Subject: [PATCH] i2psnark: Smooth out request throttling The request throttler had several problems: - Throttling was very laggy, but when it reacted it throttled too quickly. - PartialPieces got returned back to PeerCoordinator, and then back to PeerState, every few seconds, rather than continuing steady-state in PeerState - Holding the PartialPiece in the PeerCoordinator for several seconds while staying interested is poor and risks snubbing - Max request queue size was reduced far too quickly, doing so exponentially when the reduction occurred linearly - shouldRequest() called the request limiter second in the conditional, so the estimator was not decayed properly when over down bw limit - shouldRequest() used overDownBandwidthLimit() rather than _down.offer(), ensuring a very slow and laggy feedback loop. - Requests were added too quickly on unthrottle Fix these by: - Fixing and slowing max request queue size reduction to simulate an exponential reduction over time when throttled - Fixing shouldRequest() to always call offer() for both limiters - Slow addition of requests after unthrottle to simulate a linear increase over time Log tweaks also The request queue now mostly maintains a steady size, and properly meters the requests. The PartialPiece returns to the PeerCoordinator much less often, and comes back to PeerState much quicker if it does. Further tuning TODO --- .../src/org/klomp/snark/BandwidthManager.java | 7 ++- .../java/src/org/klomp/snark/PeerState.java | 43 +++++++++++++++---- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/apps/i2psnark/java/src/org/klomp/snark/BandwidthManager.java b/apps/i2psnark/java/src/org/klomp/snark/BandwidthManager.java index 0cbfb04ad..c79323f83 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/BandwidthManager.java +++ b/apps/i2psnark/java/src/org/klomp/snark/BandwidthManager.java @@ -131,7 +131,12 @@ public class BandwidthManager implements BandwidthListener { * @param peer ignored */ public boolean shouldRequest(Peer peer, int size) { - boolean rv = !overDownBWLimit() && _req.offer(size, 1.0f); + // We always call both limiters, + // so that both the Westwood estimates will be + // decayed appropriately in all cases + boolean a = _req.offer(size, 1.0f); + boolean b = _down.offer(0, 1.0f); + boolean rv = a && b; if (!rv && _log.shouldWarn()) _log.warn("Deny requesting " + size + " bytes, download rate " + DataHelper.formatSize(getDownloadRate()) + "Bps" + ", request rate " + DataHelper.formatSize(getRequestRate()) + "Bps"); diff --git a/apps/i2psnark/java/src/org/klomp/snark/PeerState.java b/apps/i2psnark/java/src/org/klomp/snark/PeerState.java index 98b0b4b6d..02bef2aec 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/PeerState.java +++ b/apps/i2psnark/java/src/org/klomp/snark/PeerState.java @@ -31,6 +31,7 @@ import java.util.Set; import net.i2p.I2PAppContext; import net.i2p.data.ByteArray; import net.i2p.util.Log; +import net.i2p.util.RandomSource; import org.klomp.snark.bencode.BEValue; import org.klomp.snark.bencode.InvalidBEncodingException; @@ -430,10 +431,10 @@ class PeerState implements DataLoader // Now reported byte-by-byte in PartialPiece //peer.downloaded(size); - if (_log.shouldLog(Log.DEBUG)) - _log.debug("got end of Chunk(" - + req.getPiece() + "," + req.off + "," + req.len + ") from " - + peer); + //if (_log.shouldLog(Log.DEBUG)) + // _log.debug("got end of Chunk(" + // + req.getPiece() + "," + req.off + "," + req.len + ") from " + // + peer); // Last chunk needed for this piece? PartialPiece pp = req.getPartialPiece(); @@ -506,10 +507,10 @@ class PeerState implements DataLoader */ Request getOutstandingRequest(int piece, int begin, int length) { - if (_log.shouldLog(Log.DEBUG)) - _log.debug("got start of Chunk(" - + piece + "," + begin + "," + length + ") from " - + peer); + //if (_log.shouldLog(Log.DEBUG)) + // _log.debug("got start of Chunk(" + // + piece + "," + begin + "," + length + ") from " + // + peer); // Lookup the correct piece chunk request from the list. Request req; @@ -912,9 +913,20 @@ class PeerState implements DataLoader // currentMaxPipeline counter. // Avoid cross-peer deadlocks from PeerCoordinator, call this outside the lock if (!bwListener.shouldRequest(peer, 0)) { + // This gets called linearly, on reception of each chunk, so + // we decrease currentMaxPipeline linearly as well. + // As reqq drops with each chunk received, we will rapidly drop the + // reqq to zero within a couple windows unless we reduce + // currentMaxPipeline more slowly, to prevent returning the + // partial piece to the coordinator and thrashing the piece + // back and forth between the peer state and the coordinator. + // And sitting idle-and-interested for a few windows which is bad. synchronized(this) { // Due to changes elsewhere we can let this go down to zero now - currentMaxPipeline /= 2; + if (currentMaxPipeline > 0) { + if (RandomSource.getInstance().nextBoolean()) + currentMaxPipeline--; + } } if (_log.shouldWarn()) _log.warn(peer + " throttle request, interesting? " + interesting + " choked? " + choked + @@ -933,6 +945,11 @@ class PeerState implements DataLoader } else if (currentMaxPipeline < 2) { currentMaxPipeline++; } + // assume 3 sec RTT, only request this much, but + // minimum 1 request (check at bottom of loop) + // to avoid filling up the request queue all at once + // and rapid throttle/unthrottle cycles + long maxReq = (limit - rate) * 3; boolean more_pieces = true; while (more_pieces) { @@ -968,6 +985,7 @@ class PeerState implements DataLoader pieceLength = metainfo.getPieceLength(lastRequest.getPiece()); isLastChunk = lastRequest.off + lastRequest.len == pieceLength; + int requested = PARTSIZE; // Last part of a piece? if (isLastChunk) { more_pieces = requestNextPiece(); @@ -980,6 +998,7 @@ class PeerState implements DataLoader int maxLength = pieceLength - nextBegin; int nextLength = maxLength > PARTSIZE ? PARTSIZE : maxLength; + requested = nextLength; Request req = new Request(nextPiece,nextBegin, nextLength); outstandingRequests.add(req); if (!choked) @@ -995,6 +1014,12 @@ class PeerState implements DataLoader } } } + if (more_pieces) { + // don't exceed the download limit calculated above + maxReq -= requested; + if (maxReq <= 0) + break; + } } } }