Skip to content

Commit

Permalink
Fix regression when throttle timeout is marginal (#2)
Browse files Browse the repository at this point in the history
* fix regression when timeout is marginal

See
tulios#1556

fixes

* always use the calculated scheduled timeout or 0

---------

Co-authored-by: MDSLKTR <[email protected]>
  • Loading branch information
terebentina and MDSLKTR authored Jun 9, 2023
1 parent 6e68af6 commit 5624cfa
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 9 deletions.
22 changes: 14 additions & 8 deletions src/network/requestQueue/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const PRIVATE = {
}

const REQUEST_QUEUE_EMPTY = 'requestQueueEmpty'
const CHECK_PENDING_REQUESTS_INTERVAL = 10

module.exports = class RequestQueue extends EventEmitter {
/**
Expand Down Expand Up @@ -56,6 +55,13 @@ module.exports = class RequestQueue extends EventEmitter {
*/
this.throttledUntil = -1

/**
* Current timestamp when the throttling was set
*
* @type {number}
*/
this.throttleCurrentTimestamp = 0

/**
* Timeout id if we have scheduled a check for pending requests due to client-side throttling
*
Expand Down Expand Up @@ -105,7 +111,8 @@ module.exports = class RequestQueue extends EventEmitter {
maybeThrottle(clientSideThrottleTime) {
if (clientSideThrottleTime !== null && clientSideThrottleTime > 0) {
this.logger.debug(`Client side throttling in effect for ${clientSideThrottleTime}ms`)
const minimumThrottledUntil = Date.now() + clientSideThrottleTime
this.throttleCurrentTimestamp = Date.now()
const minimumThrottledUntil = this.throttleCurrentTimestamp + clientSideThrottleTime
this.throttledUntil = Math.max(minimumThrottledUntil, this.throttledUntil)
}
}
Expand Down Expand Up @@ -309,15 +316,14 @@ module.exports = class RequestQueue extends EventEmitter {
// will be fine, and potentially fix up a new timeout if needed at that time.
// Note that if we're merely "overloaded" by having too many inflight requests
// we will anyways check the queue when one of them gets fulfilled.
let scheduleAt = this.throttledUntil - Date.now()
if (!this.throttleCheckTimeoutId) {
if (this.pending.length > 0) {
scheduleAt = scheduleAt > 0 ? scheduleAt : CHECK_PENDING_REQUESTS_INTERVAL
}
const timeUntilUnthrottled = this.throttledUntil - this.throttleCurrentTimestamp

if (timeUntilUnthrottled > 0 && !this.throttleCheckTimeoutId) {
this.throttleCheckTimeoutId = setTimeout(() => {
this.throttleCheckTimeoutId = null
this.throttleCurrentTimestamp = Date.now()
this.checkPendingRequests()
}, scheduleAt)
}, Math.max(timeUntilUnthrottled, 0))
}
}
}
1 change: 0 additions & 1 deletion src/network/requestQueue/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ describe('Network > RequestQueue', () => {
const before = Date.now()
const clientSideThrottleTime = 1
requestQueue.maybeThrottle(clientSideThrottleTime)
// Sleep until the marginal delay is passed before calling scheduleCheckPendingRequests()
await sleep(clientSideThrottleTime)
requestQueue.scheduleCheckPendingRequests()

Expand Down

0 comments on commit 5624cfa

Please sign in to comment.