[PictureLoader] Seed per-host allowances on demand and skip hosts in 429 backoff

The quota reset re-filled every host's remaining allowance to a full
MAX_REQUESTS_PER_SEC as soon as the queue had a request for it, so a server
that was just rate limited could be hammered again at full speed immediately
after (or even during) recovery.

Only seed a host's allowance the first time it is dispatched in the current
second, seeded from its reduced sustained quota, and skip hosts still inside
their 429 backoff window entirely, handing the entry back to its worker so it
can wait the backoff out or fall through to another source instead of parking
in the queue with no reply pending. Deferrals wait on the host that is
actually blocking the request (cached-redirect targets and the reply host of
a 429) rather than the current card URL's host.

Rebased onto network-requests/request-pacing, which absorbed the earlier
pacing and dispatch-guard commits, and reuses its updateTimerState idle-429
recovery plumbing.

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp

@@ -107,6 +107,14 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture
     // Check for cached redirects
     QUrl cachedRedirect = getCachedRedirect(url);
     if (!cachedRedirect.isEmpty()) {
+        // The redirect target is a different host, which may itself be in 429 backoff; hand the
+        // entry back to its worker so it waits the backoff out instead of dispatching straight
+        // onto the backed-off host.
+        if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(cachedRedirect.host(),
+                                                                     QDateTime::currentDateTime())) {
+            worker->scheduleDeferredRetry(cachedRedirect.host());
+            return nullptr;
+        }
         emit imageRequestSucceeded(url);
         return makeRequest(cachedRedirect, worker);
     }
@@ -143,10 +151,10 @@ void CardPictureLoaderWorker::resetRequestQuota()
         }
     }
 
-    for (const auto &request : requestLoadQueue) {
-        const QString host = request.first.host();
-        hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC));
-    }
+    // Forget the per-second allowances; each host's allowance is re-seeded lazily from its
+    // reduced sustained quota the first time it is dispatched in the new second, so a host that
+    // enters the queue mid-second no longer falls through to a fresh full quota.
+    hostQuotaRemaining.clear();
 
     updateTimerState();
 }
@@ -212,14 +220,28 @@ void CardPictureLoaderWorker::updateTimerState()
 
 bool CardPictureLoaderWorker::processSingleRequest()
 {
+    QDateTime now = QDateTime::currentDateTime();
     for (int i = 0; i < requestLoadQueue.size(); ++i) {
         const auto &request = requestLoadQueue.at(i);
-        QString host = request.first.host();
-        int allowance = hostQuotaRemaining.value(host, MAX_REQUESTS_PER_SEC);
+        const QString host = request.first.host();
+        // Don't dispatch requests to a host that is currently in its 429 backoff; hand the entry
+        // back to its worker so it can wait the backoff out or fall through to another source,
+        // instead of leaving it parked in the queue with no reply pending.
+        if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) {
+            auto entry = requestLoadQueue.takeAt(i);
+            entry.second->startNextPicDownload();
+            return true;
+        }
+        // Seed the allowance now so a host that was rate limited gets its reduced
+        // allowance instead of a fresh full quota mid-second.
+        if (!hostQuotaRemaining.contains(host)) {
+            hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC));
+        }
+        int allowance = hostQuotaRemaining.value(host);
         if (allowance > 0) {
             hostQuotaRemaining.insert(host, allowance - 1);
-            makeRequest(request.first, request.second);
-            requestLoadQueue.removeAt(i);
+            auto entry = requestLoadQueue.takeAt(i);
+            makeRequest(entry.first, entry.second);
             return true;
         }
     }

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp

@@ -22,6 +22,11 @@ static const QStringList MD5_BLACKLIST = {
     "fbc7d763c08771c260b39e2115414eeb"  // Current card back hash
 };
 
+const ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter()
+{
+    return s_rateLimiter;
+}
+
 CardPictureLoaderWorkerWork::CardPictureLoaderWorkerWork(const CardPictureLoaderWorker *worker, const ExactCard &toLoad)
     : QObject(nullptr), cardToDownload(CardPictureToLoad(toLoad)),
       picDownload(SettingsCache::instance().downloads().getPicDownload())
@@ -168,7 +173,7 @@ void CardPictureLoaderWorkerWork::handleFailedReply(const QNetworkReply *reply)
                 << "PictureLoader: [card: " << cardToDownload.getCard().getName()
                 << " set: " << cardToDownload.getSetName() << "]: Too many requests from " << host
                 << ", backing off until " << backoffUntil.toString(Qt::ISODate) << ", retrying the same url";
-            scheduleDeferredRetry();
+            scheduleDeferredRetry(host);
         } else {
             qCWarning(CardPictureLoaderWorkerWorkLog).nospace()
                 << "PictureLoader: [card: " << cardToDownload.getCard().getName()
@@ -273,14 +278,16 @@ QImage CardPictureLoaderWorkerWork::tryLoadImageFromReply(QNetworkReply *reply)
     return imgReader.read();
 }
 
-void CardPictureLoaderWorkerWork::scheduleDeferredRetry()
+void CardPictureLoaderWorkerWork::scheduleDeferredRetry(const QString &preferredHost)
 {
     QDateTime now = QDateTime::currentDateTime();
 
-    // Prefer waiting on the current URL's server so we retry the same source.
-    QString currentHost = QUrl(cardToDownload.getCurrentUrl()).host();
-    QDateTime backoffUntil = s_rateLimiter.deadline(currentHost);
-    if (!s_rateLimiter.isRateLimited(currentHost, now)) {
+    // Prefer waiting on the server that is actually blocking the request: callers hand in the
+    // rate-limited host when it differs from the current URL (e.g. a cached redirect target still
+    // in backoff), otherwise fall back to the current URL's server so we retry the same source.
+    QString waitHost = preferredHost.isEmpty() ? QUrl(cardToDownload.getCurrentUrl()).host() : preferredHost;
+    QDateTime backoffUntil = s_rateLimiter.deadline(waitHost);
+    if (!s_rateLimiter.isRateLimited(waitHost, now)) {
         backoffUntil = s_rateLimiter.earliestDeadline(now);
     }
 

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h

@@ -10,6 +10,7 @@
 #include <QNetworkAccessManager>
 #include <QObject>
 #include <QRandomGenerator>
+#include <QString>
 #include <QThread>
 #include <libcockatrice/card/database/card_database.h>
 #include <libcockatrice/utility/server_rate_limiter.h>
@@ -43,6 +44,30 @@ public:
 
     CardPictureToLoad cardToDownload; ///< The card and associated URLs to try downloading
 
+    /** @brief Shared per-server 429 backoff state. */
+    static const ServerRateLimiter &rateLimiter();
+
+    /**
+     * @brief Starts downloading the next URL for this card.
+     *
+     * Skips URLs whose server is currently in 429 backoff, either waiting the
+     * backoff out or falling through to the other configured sources. Also used by
+     * the dispatch machinery to hand an entry back after it was removed from the
+     * request queue when its host turned out to be backed off.
+     */
+    void startNextPicDownload();
+
+    /**
+     * @brief Schedules a deferred retry after the relevant server backoff expires.
+     * @param preferredHost The server that is actually blocking the request, or an empty
+     *                      string to use the current URL's server
+     *
+     * Waits on the blocking server's backoff deadline, otherwise on the earliest active
+     * backoff. If no servers are in backoff, concludes with failure. Otherwise resets the
+     * CardPictureToLoad indices and retries after the backoff period.
+     */
+    void scheduleDeferredRetry(const QString &preferredHost = {});
+
 public slots:
     /**
      * @brief Handles a finished network reply for the card image.
@@ -55,9 +80,6 @@ private:
 
     static ServerRateLimiter s_rateLimiter; ///< Shared per-server 429 backoff state
 
-    /** @brief Starts downloading the next URL for this card. */
-    void startNextPicDownload();
-
     /** @brief Called when all URLs have been exhausted or download failed. */
     void picDownloadFailed();
 
@@ -82,16 +104,6 @@ private:
      */
     void concludeImageLoad(const QImage &image);
 
-    /**
-     * @brief Schedules a deferred retry after the relevant server backoff expires.
-     *
-     * Waits on the current URL's server when it is the reason we are blocked,
-     * otherwise on the earliest active backoff. If no servers are in backoff,
-     * concludes with failure. Otherwise resets the CardPictureToLoad indices and
-     * retries after the backoff period.
-     */
-    void scheduleDeferredRetry();
-
 private slots:
     /** @brief Updates the picDownload setting when it changes. */
     void picDownloadChanged();