From 310caa7dc0694f3cddc4a73f8d53b532440fc827 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Fri, 18 Sep 2026 04:21:09 +0200 Subject: [PATCH] [PictureLoader] Hand backed-off requests back to their worker instead of parking them --- .../card_picture_loader_worker.cpp | 25 +++++++++---- .../card_picture_loader_worker_work.cpp | 2 +- .../card_picture_loader_worker_work.h | 35 +++++++++++-------- 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp index 91a984dea..2925b3af9 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp @@ -108,6 +108,14 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture QUrl cachedRedirect = getCachedRedirect(url); if (!cachedRedirect.isEmpty()) { emit imageRequestSucceeded(url); + // 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(); + return nullptr; + } return makeRequest(cachedRedirect, worker); } @@ -136,8 +144,9 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture void CardPictureLoaderWorker::resetRequestQuota() { - // Allowances are seeded per host on demand in processSingleRequest(), so a - // rate-limited host never gets a fresh full quota mid-second. + // Allowances are seeded lazily per host in processSingleRequest() when a request is first + // looked at in a new second, so a host that enters the queue mid-second now gets its reduced + // per-host allowance instead of falling through to the full per-second default. hostQuotaRemaining.clear(); QDateTime now = QDateTime::currentDateTime(); @@ -191,12 +200,16 @@ bool CardPictureLoaderWorker::processSingleRequest() for (int i = 0; i < requestLoadQueue.size(); ++i) { const auto &request = requestLoadQueue.at(i); const QString host = request.first.host(); - // Don't dispatch requests to a host that is currently in its 429 backoff. + // 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)) { - continue; + requestLoadQueue.removeAt(i); + request.second->startNextPicDownload(); + return true; } - // Seed the allowance only now, so a host that was rate limited last second - // doesn't get a fresh full quota the moment it is queried mid-second. + // 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)); } diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp index 072a919d7..e4a36ab8c 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp @@ -22,7 +22,7 @@ static const QStringList MD5_BLACKLIST = { "fbc7d763c08771c260b39e2115414eeb" // Current card back hash }; -ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter() +const ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter() { return s_rateLimiter; } diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h index 8490cb3ac..f05d727de 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h @@ -44,7 +44,27 @@ public: CardPictureToLoad cardToDownload; ///< The card and associated URLs to try downloading /** @brief Shared per-server 429 backoff state. */ - static ServerRateLimiter &rateLimiter(); + 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. + * + * 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(); public slots: /** @@ -58,9 +78,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(); @@ -85,16 +102,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();