[PictureLoader] Reconcile quota-timer lifecycle with idle 429 recovery

This commit is contained in:
Lukas Brübach 2026-09-20 21:24:40 +02:00 • committed by GitHub
parent 8cd6d981ed
commit b747407d8d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 46 additions and 19 deletions

View file

@ -148,35 +148,25 @@ void CardPictureLoaderWorker::resetRequestQuota()
hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC)); hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC));
} }
processQueuedRequests(); updateTimerState();
} }
void CardPictureLoaderWorker::processQueuedRequests() void CardPictureLoaderWorker::processQueuedRequests()
{ {
Q_ASSERT(thread() == QThread::currentThread()); // QTimer must be started from the thread it lives in; if this public slot is ever reached from
// another thread, replay it on the worker's event loop instead of letting start() fail silently.
if (requestLoadQueue.isEmpty()) { if (thread() != QThread::currentThread()) {
dispatchTimer.stop(); QMetaObject::invokeMethod(this, &CardPictureLoaderWorker::processQueuedRequests, Qt::QueuedConnection);
requestTimer.stop();
return; return;
} }
// Start lazily from the worker's own thread: QTimer must be started in the thread it lives in. updateTimerState();
if (!requestTimer.isActive()) {
requestTimer.start();
}
// Restarting an active timer would reset the pacing countdown, so a burst of enqueues could
// keep starving the dispatcher; only start it when it has actually stopped.
if (!dispatchTimer.isActive()) {
dispatchTimer.start();
}
} }
void CardPictureLoaderWorker::dispatchQueuedRequest() void CardPictureLoaderWorker::dispatchQueuedRequest()
{ {
if (requestLoadQueue.isEmpty()) { if (requestLoadQueue.isEmpty()) {
// All queued requests have been dispatched; stop the pacing and quota-reset timers. // All queued requests have been dispatched; stop the pacing timers.
dispatchTimer.stop(); updateTimerState();
requestTimer.stop();
return; return;
} }
@ -186,6 +176,40 @@ void CardPictureLoaderWorker::dispatchQueuedRequest()
} }
} }
void CardPictureLoaderWorker::updateTimerState()
{
// Never restart an active timer: that would reset the pacing countdown and a burst of enqueues
// could keep starving the dispatcher, so only (re)start a timer that has actually stopped.
if (requestLoadQueue.isEmpty()) {
dispatchTimer.stop();
// Forget per-second allowances once nothing is pending: a stale zero would otherwise delay
// the next single request by a full quota-reset interval.
hostQuotaRemaining.clear();
} else if (!dispatchTimer.isActive()) {
dispatchTimer.start();
}
// The quota timer resets allowances every second and is also the only thing that heals a host
// after a 429 (see resetRequestQuota). It must keep ticking while work is queued or a host is
// still recovering below the ceiling, and only winds down once no host needs recovery anymore.
// Keeping it alive during such idle periods lets reduced quotas recover as intended.
bool hostRecovering = false;
for (auto it = hostRequestQuota.cbegin(); it != hostRequestQuota.cend(); ++it) {
if (it.value() < MAX_REQUESTS_PER_SEC) {
hostRecovering = true;
break;
}
}
if (!requestLoadQueue.isEmpty() || hostRecovering) {
if (!requestTimer.isActive()) {
requestTimer.start();
}
} else if (requestTimer.isActive()) {
requestTimer.stop();
}
}
bool CardPictureLoaderWorker::processSingleRequest() bool CardPictureLoaderWorker::processSingleRequest()
{ {
for (int i = 0; i < requestLoadQueue.size(); ++i) { for (int i = 0; i < requestLoadQueue.size(); ++i) {

View file

@ -86,7 +86,7 @@ public slots:
*/ */
QNetworkReply *makeRequest(const QUrl &url, CardPictureLoaderWorkerWork *workThread); QNetworkReply *makeRequest(const QUrl &url, CardPictureLoaderWorkerWork *workThread);
/** @brief Starts the pacing timers if there is queued work, stops them when the queue is empty. */ /** @brief Ensures the pacing and quota-reset timers reflect the current queue and recovery state. */
void processQueuedRequests(); void processQueuedRequests();
/** @brief Chooses a request from the queue and starts it, respecting the quota and pacing. */ /** @brief Chooses a request from the queue and starts it, respecting the quota and pacing. */
@ -142,6 +142,9 @@ private:
/** @brief Removes stale redirect entries older than TTL. */ /** @brief Removes stale redirect entries older than TTL. */
void cleanStaleEntries(); void cleanStaleEntries();
/** @brief Starts or stops the pacing and quota-reset timers to match the queue and recovery state. */
void updateTimerState();
private slots: private slots:
/** @brief Resets the request quota for rate-limiting. */ /** @brief Resets the request quota for rate-limiting. */
void resetRequestQuota(); void resetRequestQuota();