From 147d38a2a498d55c81a5a03b4bf8b2ac26c658b3 Mon Sep 17 00:00:00 2001
From: Sergei Zharinov <zharinov@users.noreply.github.com>
Date: Tue, 5 Mar 2024 11:50:09 -0300
Subject: [PATCH] feat: Limit HTTP concurrency and frequency by default
(#27621)
---
lib/config/options/index.ts | 8 ++---
lib/util/http/host-rules.spec.ts | 51 +++++++++++++++++++++++++++-
lib/util/http/host-rules.ts | 36 +++++++++++++++-----
lib/util/http/index.spec.ts | 58 +++++++++++++++++++++++++-------
lib/util/http/index.ts | 12 +++++--
lib/util/http/queue.spec.ts | 10 +++---
lib/util/http/queue.ts | 10 ++++--
lib/util/http/throttle.spec.ts | 10 +++---
lib/util/http/throttle.ts | 10 ++++--
lib/workers/global/initialize.ts | 2 ++
10 files changed, 166 insertions(+), 41 deletions(-)
diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts
index 466a74d50d..6f9c5cfe11 100644
--- a/lib/config/options/index.ts
+++ b/lib/config/options/index.ts
@@ -2416,21 +2416,21 @@ const options: RenovateOptions[] = [
},
{
name: 'concurrentRequestLimit',
- description: 'Limit concurrent requests per host.',
+ description: 'Limit concurrent requests per host. Set to 0 for no limit.',
type: 'integer',
+ default: 5,
stage: 'repository',
parents: ['hostRules'],
- default: null,
cli: false,
env: false,
},
{
name: 'maxRequestsPerSecond',
- description: 'Limit requests rate per host.',
+ description: 'Limit requests rate per host. Set to 0 for no limit.',
type: 'integer',
+ default: 5,
stage: 'repository',
parents: ['hostRules'],
- default: 0,
cli: false,
env: false,
},
diff --git a/lib/util/http/host-rules.spec.ts b/lib/util/http/host-rules.spec.ts
index 2b45213283..d558558629 100644
--- a/lib/util/http/host-rules.spec.ts
+++ b/lib/util/http/host-rules.spec.ts
@@ -3,7 +3,12 @@ import { bootstrap } from '../../proxy';
import type { HostRule } from '../../types';
import * as hostRules from '../host-rules';
import { dnsLookup } from './dns';
-import { applyHostRule, findMatchingRule } from './host-rules';
+import {
+ applyHostRule,
+ findMatchingRule,
+ getConcurrentRequestsLimit,
+ getThrottleIntervalMs,
+} from './host-rules';
import type { GotOptions } from './types';
const url = 'https://github.com';
@@ -560,4 +565,48 @@ describe('util/http/host-rules', () => {
},
});
});
+
+ describe('getConcurrentRequestsLimit', () => {
+ it('returns default value for undefined rule', () => {
+ expect(getConcurrentRequestsLimit('https://example.com', 42)).toBe(42);
+ });
+
+ it('returns null for 0', () => {
+ hostRules.add({
+ matchHost: 'https://example.com',
+ concurrentRequestLimit: 0,
+ });
+ expect(getConcurrentRequestsLimit('https://example.com', 42)).toBeNull();
+ });
+
+ it('returns positive limit', () => {
+ hostRules.add({
+ matchHost: 'https://example.com',
+ concurrentRequestLimit: 143,
+ });
+ expect(getConcurrentRequestsLimit('https://example.com', 42)).toBe(143);
+ });
+ });
+
+ describe('getThrottleIntervalMs', () => {
+ it('returns default value for undefined rule', () => {
+ expect(getThrottleIntervalMs('https://example.com', 42)).toBe(24); // 1000 / 42
+ });
+
+ it('returns null for 0', () => {
+ hostRules.add({
+ matchHost: 'https://example.com',
+ maxRequestsPerSecond: 0,
+ });
+ expect(getThrottleIntervalMs('https://example.com', 42)).toBeNull();
+ });
+
+ it('returns positive limit', () => {
+ hostRules.add({
+ matchHost: 'https://example.com',
+ maxRequestsPerSecond: 143,
+ });
+ expect(getThrottleIntervalMs('https://example.com', 42)).toBe(7); // 1000 / 143
+ });
+ });
});
diff --git a/lib/util/http/host-rules.ts b/lib/util/http/host-rules.ts
index 20c8981037..54387c4ec3 100644
--- a/lib/util/http/host-rules.ts
+++ b/lib/util/http/host-rules.ts
@@ -217,16 +217,36 @@ export function applyHostRule<GotOptions extends HostRulesGotOptions>(
return options;
}
-export function getConcurrentRequestsLimit(url: string): number | null {
+export function getConcurrentRequestsLimit(
+ url: string,
+ defaultValue: number | null,
+): number | null {
const { concurrentRequestLimit } = hostRules.find({ url });
- return is.number(concurrentRequestLimit) && concurrentRequestLimit > 0
- ? concurrentRequestLimit
- : null;
+
+ if (!is.number(concurrentRequestLimit)) {
+ return defaultValue;
+ }
+
+ if (concurrentRequestLimit > 0) {
+ return concurrentRequestLimit;
+ }
+
+ return null;
}
-export function getThrottleIntervalMs(url: string): number | null {
+export function getThrottleIntervalMs(
+ url: string,
+ defaultValue: number | null,
+): number | null {
const { maxRequestsPerSecond } = hostRules.find({ url });
- return is.number(maxRequestsPerSecond) && maxRequestsPerSecond > 0
- ? Math.ceil(1000 / maxRequestsPerSecond)
- : null;
+
+ if (!is.number(maxRequestsPerSecond)) {
+ return defaultValue ? Math.ceil(1000 / defaultValue) : defaultValue; // 5 requests per second
+ }
+
+ if (maxRequestsPerSecond > 0) {
+ return Math.ceil(1000 / maxRequestsPerSecond);
+ }
+
+ return null;
}
diff --git a/lib/util/http/index.spec.ts b/lib/util/http/index.spec.ts
index 4541f6b86a..453cbc032c 100644
--- a/lib/util/http/index.spec.ts
+++ b/lib/util/http/index.spec.ts
@@ -516,34 +516,68 @@ describe('util/http/index', () => {
});
describe('Throttling', () => {
+ const delta = 100;
+
+ beforeEach(() => {
+ jest.useFakeTimers({ advanceTimers: true });
+ });
+
afterEach(() => {
jest.useRealTimers();
});
it('works without throttling', async () => {
- jest.useFakeTimers({ advanceTimers: 1 });
- httpMock.scope(baseUrl).get('/foo').twice().reply(200, 'bar');
+ httpMock.scope(baseUrl).get('/foo').times(10).reply(200, 'bar');
const t1 = Date.now();
- await http.get('http://renovate.com/foo');
- await http.get('http://renovate.com/foo');
+ await Promise.all([
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ ]);
const t2 = Date.now();
- expect(t2 - t1).toBeLessThan(100);
+ expect(t2 - t1).toBeLessThan(delta);
});
it('limits request rate by host', async () => {
- jest.useFakeTimers({ advanceTimers: true });
- httpMock.scope(baseUrl).get('/foo').twice().reply(200, 'bar');
- hostRules.add({ matchHost: 'renovate.com', maxRequestsPerSecond: 0.25 });
+ httpMock.scope(baseUrl).get('/foo').times(4).reply(200, 'bar');
+ hostRules.add({ matchHost: 'renovate.com', maxRequestsPerSecond: 3 });
+
+ const t1 = Date.now();
+ await Promise.all([
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ ]);
+ const t2 = Date.now();
+
+ expect(t2 - t1).toBeWithin(1000, 1000 + delta);
+ });
+
+ it('defaults to 5 requests per second', async () => {
+ Http.setDefaultLimits();
+ httpMock.scope(baseUrl).get('/foo').times(5).reply(200, 'bar');
const t1 = Date.now();
- await http.get('http://renovate.com/foo');
- jest.advanceTimersByTime(4000);
- await http.get('http://renovate.com/foo');
+ await Promise.all([
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ http.get('http://renovate.com/foo'),
+ ]);
const t2 = Date.now();
- expect(t2 - t1).toBeGreaterThanOrEqual(4000);
+ expect(t2 - t1).toBeWithin(800, 800 + delta);
});
});
diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts
index c58279d421..920620fb10 100644
--- a/lib/util/http/index.ts
+++ b/lib/util/http/index.ts
@@ -126,6 +126,14 @@ async function gotTask<T>(
}
export class Http<Opts extends HttpOptions = HttpOptions> {
+ private static defaultConcurrentRequestLimit: number | null = null;
+ private static defaultMaxRequestsPerSecond: number | null = null;
+
+ static setDefaultLimits(): void {
+ Http.defaultConcurrentRequestLimit = 5;
+ Http.defaultMaxRequestsPerSecond = 5;
+ }
+
private options?: GotOptions;
constructor(
@@ -143,7 +151,7 @@ export class Http<Opts extends HttpOptions = HttpOptions> {
}
protected getThrottle(url: string): Throttle | null {
- return getThrottle(url);
+ return getThrottle(url, Http.defaultMaxRequestsPerSecond);
}
protected async request<T>(
@@ -249,7 +257,7 @@ export class Http<Opts extends HttpOptions = HttpOptions> {
? () => throttle.add<HttpResponse<T>>(httpTask)
: httpTask;
- const queue = getQueue(url);
+ const queue = getQueue(url, Http.defaultConcurrentRequestLimit);
const queuedTask: GotTask<T> = queue
? () => queue.add<HttpResponse<T>>(throttledTask)
: throttledTask;
diff --git a/lib/util/http/queue.spec.ts b/lib/util/http/queue.spec.ts
index 6906952239..ac8f12c0d5 100644
--- a/lib/util/http/queue.spec.ts
+++ b/lib/util/http/queue.spec.ts
@@ -12,15 +12,15 @@ describe('util/http/queue', () => {
});
it('returns null for invalid URL', () => {
- expect(getQueue('$#@!')).toBeNull();
+ expect(getQueue('$#@!', null)).toBeNull();
});
it('returns queue for valid url', () => {
- const q1a = getQueue('https://example.com');
- const q1b = getQueue('https://example.com');
+ const q1a = getQueue('https://example.com', null);
+ const q1b = getQueue('https://example.com', null);
- const q2a = getQueue('https://example.com:8080');
- const q2b = getQueue('https://example.com:8080');
+ const q2a = getQueue('https://example.com:8080', null);
+ const q2b = getQueue('https://example.com:8080', null);
expect(q1a).not.toBeNull();
expect(q1a).toBe(q1b);
diff --git a/lib/util/http/queue.ts b/lib/util/http/queue.ts
index 1c127d2f26..44beff21ef 100644
--- a/lib/util/http/queue.ts
+++ b/lib/util/http/queue.ts
@@ -5,7 +5,10 @@ import { getConcurrentRequestsLimit } from './host-rules';
const hostQueues = new Map<string, PQueue | null>();
-export function getQueue(url: string): PQueue | null {
+export function getQueue(
+ url: string,
+ defaultConcurrentRequestLimit: number | null,
+): PQueue | null {
const host = parseUrl(url)?.host;
if (!host) {
// should never happen
@@ -16,7 +19,10 @@ export function getQueue(url: string): PQueue | null {
let queue = hostQueues.get(host);
if (queue === undefined) {
queue = null; // null represents "no queue", as opposed to undefined
- const concurrency = getConcurrentRequestsLimit(url);
+ const concurrency = getConcurrentRequestsLimit(
+ url,
+ defaultConcurrentRequestLimit,
+ );
if (concurrency) {
logger.debug(`Using queue: host=${host}, concurrency=${concurrency}`);
queue = new PQueue({ concurrency });
diff --git a/lib/util/http/throttle.spec.ts b/lib/util/http/throttle.spec.ts
index 5163f23def..d0e3a2f1ce 100644
--- a/lib/util/http/throttle.spec.ts
+++ b/lib/util/http/throttle.spec.ts
@@ -12,15 +12,15 @@ describe('util/http/throttle', () => {
});
it('returns null for invalid URL', () => {
- expect(getThrottle('$#@!')).toBeNull();
+ expect(getThrottle('$#@!', null)).toBeNull();
});
it('returns throttle for valid url', () => {
- const t1a = getThrottle('https://example.com');
- const t1b = getThrottle('https://example.com');
+ const t1a = getThrottle('https://example.com', null);
+ const t1b = getThrottle('https://example.com', null);
- const t2a = getThrottle('https://example.com:8080');
- const t2b = getThrottle('https://example.com:8080');
+ const t2a = getThrottle('https://example.com:8080', null);
+ const t2b = getThrottle('https://example.com:8080', null);
expect(t1a).not.toBeNull();
expect(t1a).toBe(t1b);
diff --git a/lib/util/http/throttle.ts b/lib/util/http/throttle.ts
index c868354ce5..995a6c0765 100644
--- a/lib/util/http/throttle.ts
+++ b/lib/util/http/throttle.ts
@@ -22,7 +22,10 @@ export class Throttle {
}
}
-export function getThrottle(url: string): Throttle | null {
+export function getThrottle(
+ url: string,
+ defaultMaxRequestsPerSecond: number | null,
+): Throttle | null {
const host = parseUrl(url)?.host;
if (!host) {
// should never happen
@@ -33,7 +36,10 @@ export function getThrottle(url: string): Throttle | null {
let throttle = hostThrottles.get(host);
if (throttle === undefined) {
throttle = null; // null represents "no throttle", as opposed to undefined
- const throttleOptions = getThrottleIntervalMs(url);
+ const throttleOptions = getThrottleIntervalMs(
+ url,
+ defaultMaxRequestsPerSecond,
+ );
if (throttleOptions) {
const intervalMs = throttleOptions;
logger.debug(`Using throttle ${intervalMs} intervalMs for host ${host}`);
diff --git a/lib/workers/global/initialize.ts b/lib/workers/global/initialize.ts
index 47c780b4ea..d2b5d0272f 100644
--- a/lib/workers/global/initialize.ts
+++ b/lib/workers/global/initialize.ts
@@ -10,6 +10,7 @@ import * as packageCache from '../../util/cache/package';
import { setEmojiConfig } from '../../util/emoji';
import { validateGitVersion } from '../../util/git';
import * as hostRules from '../../util/host-rules';
+import { Http } from '../../util/http';
import { initMergeConfidence } from '../../util/merge-confidence';
import { setMaxLimit } from './limits';
@@ -79,6 +80,7 @@ export async function globalInitialize(
config_: AllConfig,
): Promise<RenovateConfig> {
let config = config_;
+ Http.setDefaultLimits();
await checkVersions();
setGlobalHostRules(config);
config = await initPlatform(config);
--
GitLab