[PictureLoader] Cap unlocked host bursts and adapt them to 429s

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp

@@ -140,8 +140,16 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture
 
     QNetworkReply *reply = networkManager->get(req);
 
+    // Track in-flight replies per host so the unlocked fast path can bound how many requests it
+    // issues at once, instead of creating replies that time out before Qt opens a connection.
+    const QString host = url.host();
+    hostInFlight.insert(host, hostInFlight.value(host) + 1);
+
     // Connect reply handling
-    connect(reply, &QNetworkReply::finished, worker, [reply, worker] { worker->handleNetworkReply(reply); });
+    connect(reply, &QNetworkReply::finished, worker, [this, reply, worker, host] {
+        hostInFlight.insert(host, qMax(0, hostInFlight.value(host) - 1));
+        worker->handleNetworkReply(reply);
+    });
 
     return reply;
 }
@@ -150,17 +158,21 @@ void CardPictureLoaderWorker::resetRequestQuota()
 {
     QDateTime now = QDateTime::currentDateTime();
     for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end();) {
-        // An unlocked host has no per-host allowance; drop any stale entry instead of
+        if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) {
-        // recovering it towards the UNLIMITED_HOST_QUOTA sentinel, which would poison it.
             if (hostAllowanceCeiling(it.key()) == DownloadSettings::UNLIMITED_HOST_QUOTA) {
+                // A developer-unlocked host that fell back after a 429 recovers towards the default
+                // allowance; once it gets there it becomes unlocked (fast-path) again.
+                if (it.value() + 1 >= DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT) {
                     it = hostRequestQuota.erase(it);
                     continue;
                 }
-        if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) {
+                it.value() += 1;
+            } else {
                 // Recover towards the host's effective allowance ceiling, which may be
                 // lowered by the user's per-host request limits.
                 it.value() = qMin(hostAllowanceCeiling(it.key()), it.value() + 1);
             }
+        }
         ++it;
     }
 
@@ -191,20 +203,34 @@ void CardPictureLoaderWorker::dispatchQueuedRequest()
         return;
     }
 
-    // Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the dispatch pacing: dispatch every
-    // queued request for them back-to-back, bounded only by their 429 backoff window and Qt's
-    // per-host connection pool.
     QDateTime now = QDateTime::currentDateTime();
+    bool dispatched = false;
+    // Set while an unlocked host still has queued work blocked only by the in-flight cap; the
+    // timer must keep running so it gets another try as soon as a slot frees. A host blocked by
+    // its 429 backoff instead waits for the next quota-reset tick to restart the dispatcher.
+    bool unlockedCapped = false;
+
+    // Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the pacing and the per-host
+    // allowance: dispatch their queued requests back-to-back, bounded by their 429 backoff and the
+    // per-host in-flight cap so a large burst can't queue replies that time out before Qt opens a
+    // connection for them.
     for (int i = 0; i < requestLoadQueue.size();) {
         const auto &request = requestLoadQueue.at(i);
         const QString host = request.first.host();
-        if (hostAllowanceCeiling(host) == DownloadSettings::UNLIMITED_HOST_QUOTA &&
+        if (isUnlockedHost(host)) {
-            !CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) {
+            if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) {
+                ++i;
+                continue;
+            }
+            if (hostInFlight.value(host) < MAX_IN_FLIGHT_PER_HOST) {
                 makeRequest(request.first, request.second);
                 requestLoadQueue.removeAt(i);
-        } else {
+                dispatched = true;
-            ++i;
+                continue;
             }
+            unlockedCapped = true;
+        }
+        ++i;
     }
 
     if (requestLoadQueue.isEmpty()) {
@@ -213,8 +239,13 @@ void CardPictureLoaderWorker::dispatchQueuedRequest()
         return;
     }
 
-    if (!processSingleRequest()) {
+    if (processSingleRequest()) {
-        // No queued host currently has allowance left in this second; wait for the quota reset.
+        dispatched = true;
+    }
+
+    // Keep the timer running while there is progress to make or unlocked work waiting on a free
+    // in-flight slot; otherwise no host has allowance left this second, so wait for the quota reset.
+    if (!dispatched && !unlockedCapped) {
         dispatchTimer.stop();
     }
 }
@@ -267,7 +298,17 @@ bool CardPictureLoaderWorker::processSingleRequest()
             entry.second->startNextPicDownload();
             return true;
         }
-        const int ceiling = hostAllowanceCeiling(host);
+        // Unlocked hosts are handled by dispatchQueuedRequest's fast path, bounded by the in-flight
+        // cap; they must not fall through to the per-host allowance arithmetic below.
+        if (isUnlockedHost(host)) {
+            continue;
+        }
+        int ceiling = hostAllowanceCeiling(host);
+        if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) {
+            // A 429 dropped this unlocked host out of the fast path and installed a concrete
+            // allowance; pace it against that allowance until the recovery loop unlocks it again.
+            ceiling = hostRequestQuota.value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT);
+        }
         // Seed the allowance lazily so a host that enters the queue mid-second gets its reduced
         // per-host allowance, clamped against the ceiling so a lowered user cap applies from this
         // second onward.
@@ -295,15 +336,20 @@ int CardPictureLoaderWorker::hostAllowanceCeiling(const QString &host) const
     return SettingsCache::instance().downloads().clampHostRequestLimit(host, requested);
 }
 
+bool CardPictureLoaderWorker::isUnlockedHost(const QString &host) const
+{
+    return hostAllowanceCeiling(host) == DownloadSettings::UNLIMITED_HOST_QUOTA && !hostRequestQuota.contains(host);
+}
+
 void CardPictureLoaderWorker::onHostRateLimited(const QString &host)
 {
     const int ceiling = hostAllowanceCeiling(host);
-    if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) {
+    // An unlocked host has no per-host allowance to halve. Install one instead so it drops out of
-        // Unlocked hosts have no per-host allowance to halve; the shared backoff
+    // the unlocked fast path and is paced like a throttled host; the recovery loop in
-        // window tracked by the rate limiter still paces them.
+    // resetRequestQuota() then walks it back up and unlocks it again.
-        return;
+    const int base =
-    }
+        ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA ? DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT : ceiling;
-    hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, ceiling) / 2));
+    hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, base) / 2));
     hostLast429.insert(host, QDateTime::currentDateTime());
 }
 

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h

@@ -127,6 +127,10 @@ private:
     QHash<QString, int> hostRequestLimits;  ///< User-set per-host request allowances
     QHash<QString, int> hostQuotaRemaining; ///< Per-host allowance left in the current second
     QHash<QString, QDateTime> hostLast429;  ///< When each host was last rate limited
+    QHash<QString, int> hostInFlight;       ///< Network replies currently in flight, per host
+
+    /** @brief Maximum concurrent in-flight network replies per host. */
+    static constexpr int MAX_IN_FLIGHT_PER_HOST = 6;
 
     CardPictureLoaderLocal *localLoader; ///< Loader for local images
     QSet<QString> currentlyLoading;      ///< Deduplication: contains pixmapCacheKey currently being loaded
@@ -139,6 +143,14 @@ private:
      */
     [[nodiscard]] int hostAllowanceCeiling(const QString &host) const;
 
+    /**
+     * @brief Whether a host may skip dispatch pacing and per-host allowance entirely.
+     *
+     * A host is unlocked while it has no user limit and no reduced allowance installed by a 429.
+     * A 429 drops it out of the fast path until resetRequestQuota() walks the allowance back up.
+     */
+    [[nodiscard]] bool isUnlockedHost(const QString &host) const;
+
     /** @brief Returns cached redirect URL for the given original URL, if available. */
     [[nodiscard]] QUrl getCachedRedirect(const QUrl &originalUrl) const;