feat: Limit HTTP concurrency and frequency by default (#27621)

renovatebot · Mar 5, 2024 · 147d38a · 147d38a
1 parent d5f3d6f
commit 147d38a
Show file tree

Hide file tree

Showing 10 changed files with 166 additions and 41 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);