diff --git a/lib/config/types.ts b/lib/config/types.ts index 7bf0bcaf0237f262b20bce2c0f4808fec3c5d682..837468400493c718b19fe24c616e4455b00fd7d4 100644 --- a/lib/config/types.ts +++ b/lib/config/types.ts @@ -271,6 +271,7 @@ export interface RenovateConfig packageFile?: string; packageRules?: PackageRule[]; postUpdateOptions?: string[]; + branchConcurrentLimit?: number | null; prConcurrentLimit?: number; prHourlyLimit?: number; forkModeDisallowMaintainerEdits?: boolean; diff --git a/lib/workers/global/limits.spec.ts b/lib/workers/global/limits.spec.ts index 84dc44687e5c4087e316019d490138406fc2d835..2a3b43997faca03deff9066ccc78f08b36eadedf 100644 --- a/lib/workers/global/limits.spec.ts +++ b/lib/workers/global/limits.spec.ts @@ -1,7 +1,13 @@ +import { partial } from '../../../test/util'; +import type { BranchConfig, BranchUpgradeConfig } from '../types'; import { + calcLimit, + hasMultipleLimits, + incCountValue, incLimitedValue, isLimitReached, resetAllLimits, + setCount, setMaxLimit, } from './limits'; @@ -60,4 +66,242 @@ describe('workers/global/limits', () => { setMaxLimit('Commits', -1000); expect(isLimitReached('Commits')).toBeTrue(); }); + + describe('calcLimit', () => { + it('handles single upgrade', () => { + const upgrades = partial<BranchUpgradeConfig>([ + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + ]); + + expect(calcLimit(upgrades, 'prHourlyLimit')).toBe(10); + expect(calcLimit(upgrades, 'branchConcurrentLimit')).toBe(11); + expect(calcLimit(upgrades, 'prConcurrentLimit')).toBe(12); + }); + + it('inherits prConcurrentLimit if branchConcurrentLimit is null', () => { + const upgrades = partial<BranchUpgradeConfig>([ + { + prHourlyLimit: 10, + branchConcurrentLimit: null, + prConcurrentLimit: 12, + }, + ]); + + expect(calcLimit(upgrades, 'prHourlyLimit')).toBe(10); + expect(calcLimit(upgrades, 'branchConcurrentLimit')).toBe(12); + expect(calcLimit(upgrades, 'prConcurrentLimit')).toBe(12); + }); + + it('returns 0 if atleast one upgrade has no limit in the branch', () => { + const upgrades = partial<BranchUpgradeConfig>([ + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + { + prHourlyLimit: 0, + branchConcurrentLimit: 0, + prConcurrentLimit: 0, + }, + { + prHourlyLimit: 1, + branchConcurrentLimit: 1, + prConcurrentLimit: 1, + }, + ]); + + expect(calcLimit(upgrades, 'prHourlyLimit')).toBe(0); + expect(calcLimit(upgrades, 'branchConcurrentLimit')).toBe(0); + expect(calcLimit(upgrades, 'prConcurrentLimit')).toBe(0); + }); + + it('computes the lowest limit if multiple limits are present', () => { + const upgrades = partial<BranchUpgradeConfig>([ + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + { + prHourlyLimit: 1, + branchConcurrentLimit: 1, + prConcurrentLimit: 1, + }, + { + prHourlyLimit: 5, + branchConcurrentLimit: 6, + prConcurrentLimit: 3, + }, + { + prHourlyLimit: 5, + branchConcurrentLimit: null, + prConcurrentLimit: undefined, + }, + { + prHourlyLimit: 5, + branchConcurrentLimit: 6, + prConcurrentLimit: 2, + }, + ]); + + expect(calcLimit(upgrades, 'prHourlyLimit')).toBe(1); + expect(calcLimit(upgrades, 'branchConcurrentLimit')).toBe(1); + expect(calcLimit(upgrades, 'prConcurrentLimit')).toBe(1); + }); + }); + + describe('hasMultipleLimits', () => { + it('handles single limit', () => { + const upgrades = partial<BranchUpgradeConfig>([ + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + ]); + expect(hasMultipleLimits(upgrades, 'prHourlyLimit')).toBe(false); + expect(hasMultipleLimits(upgrades, 'branchConcurrentLimit')).toBe(false); + expect(hasMultipleLimits(upgrades, 'prConcurrentLimit')).toBe(false); + }); + + it('returns false if there are multiple limits with value', () => { + const upgrades = partial<BranchUpgradeConfig>([ + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + ]); + expect(hasMultipleLimits(upgrades, 'prHourlyLimit')).toBe(false); + expect(hasMultipleLimits(upgrades, 'branchConcurrentLimit')).toBe(false); + expect(hasMultipleLimits(upgrades, 'prConcurrentLimit')).toBe(false); + }); + + it('handles multiple limits', () => { + const upgrades = partial<BranchUpgradeConfig>([ + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + { + prHourlyLimit: 11, + branchConcurrentLimit: 12, + prConcurrentLimit: 13, + }, + { + prHourlyLimit: 0, + branchConcurrentLimit: null, + prConcurrentLimit: 3, + }, + ]); + expect(hasMultipleLimits(upgrades, 'prHourlyLimit')).toBe(true); + expect(hasMultipleLimits(upgrades, 'branchConcurrentLimit')).toBe(true); + expect(hasMultipleLimits(upgrades, 'prConcurrentLimit')).toBe(true); + }); + }); + + describe('isLimitReached', () => { + it('returns false based on concurrent limits', () => { + setCount('ConcurrentPRs', 1); + setCount('HourlyPRs', 1); + incCountValue('Branches'); // using incCountValue so it gets test coverage + const upgrades = partial<BranchUpgradeConfig>([ + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + { + prHourlyLimit: 11, + branchConcurrentLimit: 12, + prConcurrentLimit: 13, + }, + { + prHourlyLimit: 0, + branchConcurrentLimit: null, + prConcurrentLimit: 3, + }, + ]); + expect( + isLimitReached('Branches', partial<BranchConfig>({ upgrades })), + ).toBe(false); + expect( + isLimitReached('ConcurrentPRs', partial<BranchConfig>({ upgrades })), + ).toBe(false); + }); + + it('returns true when hourly limit is reached', () => { + setCount('Branches', 2); + setCount('ConcurrentPRs', 2); + setCount('HourlyPRs', 2); + const upgrades = partial<BranchUpgradeConfig>([ + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + { + prHourlyLimit: 11, + branchConcurrentLimit: 12, + prConcurrentLimit: 13, + }, + { + prHourlyLimit: 2, + branchConcurrentLimit: null, + prConcurrentLimit: 3, + }, + ]); + expect( + isLimitReached('Branches', partial<BranchConfig>({ upgrades })), + ).toBe(true); + expect( + isLimitReached('ConcurrentPRs', partial<BranchConfig>({ upgrades })), + ).toBe(true); + }); + + it('returns true when concurrent limit is reached', () => { + setCount('Branches', 3); + setCount('ConcurrentPRs', 3); + setCount('HourlyPRs', 4); + const upgrades = partial<BranchUpgradeConfig>([ + { + prHourlyLimit: 10, + branchConcurrentLimit: 11, + prConcurrentLimit: 12, + }, + { + prHourlyLimit: 11, + branchConcurrentLimit: 12, + prConcurrentLimit: 13, + }, + { + prHourlyLimit: 5, + branchConcurrentLimit: null, + prConcurrentLimit: 3, + }, + ]); + expect( + isLimitReached('Branches', partial<BranchConfig>({ upgrades })), + ).toBe(true); + expect( + isLimitReached('ConcurrentPRs', partial<BranchConfig>({ upgrades })), + ).toBe(true); + }); + }); }); diff --git a/lib/workers/global/limits.ts b/lib/workers/global/limits.ts index aa030b059506471785b1518f99cdf0d99c85365e..628cf9534bf1ef7cf405d4aaec960497c0f46756 100644 --- a/lib/workers/global/limits.ts +++ b/lib/workers/global/limits.ts @@ -1,7 +1,8 @@ +import is from '@sindresorhus/is'; import { logger } from '../../logger'; +import type { BranchConfig, BranchUpgradeConfig } from '../types'; -export type Limit = 'Commits' | 'PullRequests' | 'Branches'; - +export type Limit = 'Commits'; interface LimitValue { max: number | null; current: number; @@ -27,8 +28,8 @@ export function incLimitedValue(key: Limit, incBy = 1): void { }); } -export function isLimitReached(key: Limit): boolean { - const limit = limits.get(key); +function handleCommitsLimit(): boolean { + const limit = limits.get('Commits'); // TODO: fix me? // eslint-disable-next-line @typescript-eslint/prefer-optional-chain if (!limit || limit.max === null) { @@ -37,3 +38,162 @@ export function isLimitReached(key: Limit): boolean { const { max, current } = limit; return max - current <= 0; } + +export type CountName = 'ConcurrentPRs' | 'HourlyPRs' | 'Branches'; + +type BranchLimitName = + | 'branchConcurrentLimit' + | 'prConcurrentLimit' + | 'prHourlyLimit'; + +export const counts = new Map<CountName, number>(); + +export function getCount(key: CountName): number { + const count = counts.get(key); + // istanbul ignore if: should not happen + if (!count) { + logger.warn(`Could not compute the count of ${key}, returning zero.`); + return 0; + } + return count; +} + +export function setCount(key: CountName, val: number): void { + counts.set(key, val); + logger.debug(`${key} count = ${val}`); +} + +export function incCountValue(key: CountName, incBy = 1): void { + const count = getCount(key); + counts.set(key, count + incBy); +} + +function handleConcurrentLimits( + key: Exclude<CountName, 'HourlyPRs'>, + config: BranchConfig, +): boolean { + const limitKey = + key === 'Branches' ? 'branchConcurrentLimit' : 'prConcurrentLimit'; + + // calculate the limits for this branch + const hourlyLimit = calcLimit(config.upgrades, 'prHourlyLimit'); + const hourlyPrCount = getCount('HourlyPRs'); + + // if a limit is defined ( >0 ) and limit reached return true ie. limit has been reached + if (hourlyLimit && hourlyPrCount >= hourlyLimit) { + return true; + } + + const limitValue = calcLimit(config.upgrades, limitKey); + const currentCount = getCount(key); + + if (limitValue && currentCount >= limitValue) { + return true; + } + + return false; +} + +export function calcLimit( + upgrades: BranchUpgradeConfig[], + limitName: BranchLimitName, +): number { + logger.debug( + { + limits: upgrades.map((upg) => { + return { depName: upg.depName, [limitName]: upg[limitName] }; + }), + }, + `${limitName} of the upgrades present in this branch`, + ); + + if (hasMultipleLimits(upgrades, limitName)) { + logger.once.debug( + `Branch has multiple ${limitName} limits. The lowest among these will be selected.`, + ); + } + + let lowestLimit = Number.MAX_SAFE_INTEGER; + for (const upgrade of upgrades) { + let limit = upgrade[limitName]; + + // inherit prConcurrentLimit value incase branchConcurrentLimit is null + if (!is.number(limit) && limitName === 'branchConcurrentLimit') { + limit = upgrade.prConcurrentLimit; + } + + // istanbul ignore if: should never happen as all limits get a default value + if (is.undefined(limit)) { + limit = Number.MAX_SAFE_INTEGER; + } + + // no limit + if (limit === 0 || limit === null) { + logger.debug( + `${limitName} of this branch is unlimited, because atleast one of the upgrade has it's ${limitName} set to "No limit" ie. 0 or null`, + ); + return 0; + } + + // limit is set + lowestLimit = limit < lowestLimit ? limit : lowestLimit; + } + + logger.debug( + `Calculated lowest ${limitName} among the upgrades present in this branch is ${lowestLimit}.`, + ); + return lowestLimit; +} + +export function hasMultipleLimits( + upgrades: BranchUpgradeConfig[], + limitName: BranchLimitName, +): boolean { + if (upgrades.length === 1) { + return false; + } + + const distinctLimits = new Set<number>(); + for (const upgrade of upgrades) { + let limitValue = upgrade[limitName]; + + // inherit prConcurrentLimit value incase branchConcurrentLimit is null + if (limitName === 'branchConcurrentLimit' && !is.number(limitValue)) { + limitValue = upgrade.prConcurrentLimit; + } + + // istanbul ignore if: should not happen as the limits are of type number + if (limitValue === null) { + limitValue = 0; + } + + if (!is.undefined(limitValue) && !distinctLimits.has(limitValue)) { + distinctLimits.add(limitValue); + } + } + + return distinctLimits.size > 1; +} + +export function isLimitReached(limit: 'Commits'): boolean; +export function isLimitReached( + limit: 'Branches' | 'ConcurrentPRs', + config: BranchConfig, +): boolean; +export function isLimitReached( + limit: 'Commits' | 'Branches' | 'ConcurrentPRs', + config?: BranchConfig, +): boolean { + if (limit === 'Commits') { + return handleCommitsLimit(); + } + + if (config) { + return handleConcurrentLimits(limit, config); + } + + // istanbul ignore next: should not happen + throw new Error( + 'Config is required for computing limits for Branches and PullRequests', + ); +} diff --git a/lib/workers/repository/process/limits.spec.ts b/lib/workers/repository/process/limits.spec.ts index 93e3810664487384ed56eee6e7984046af8aee14..7e9e7030d7abfae1571b6242aaa87a9276717a0b 100644 --- a/lib/workers/repository/process/limits.spec.ts +++ b/lib/workers/repository/process/limits.spec.ts @@ -18,8 +18,8 @@ beforeEach(() => { }); describe('workers/repository/process/limits', () => { - describe('getPrHourlyRemaining()', () => { - it('calculates hourly limit remaining', async () => { + describe('getPrHourlyCount()', () => { + it('calculates hourly pr count', async () => { const time = DateTime.local(); const createdAt = time.toISO(); platform.getPrList.mockResolvedValueOnce([ @@ -33,30 +33,19 @@ describe('workers/repository/process/limits', () => { { createdAt, sourceBranch: 'bar/configure' }, { createdAt, sourceBranch: 'baz/test' }, ] as never); - const res = await limits.getPrHourlyRemaining({ - ...config, - prHourlyLimit: 10, - }); - expect(res).toBe(7); + const res = await limits.getPrHourlyCount(config); + expect(res).toBe(3); }); - it('returns prHourlyLimit if errored', async () => { - config.prHourlyLimit = 5; + it('returns zero if errored', async () => { platform.getPrList.mockRejectedValue('Unknown error'); - const res = await limits.getPrHourlyRemaining(config); - expect(res).toBe(5); - }); - - it('returns MAX_SAFE_INTEGER if no hourly limit', async () => { - config.prHourlyLimit = 0; - const res = await limits.getPrHourlyRemaining(config); - expect(res).toBe(Number.MAX_SAFE_INTEGER); + const res = await limits.getPrHourlyCount(config); + expect(res).toBe(0); }); }); - describe('getConcurrentPrsRemaining()', () => { - it('calculates concurrent limit remaining', async () => { - config.prConcurrentLimit = 20; + describe('getConcurrentPrsCount()', () => { + it('calculates concurrent prs present', async () => { platform.getBranchPr.mockImplementation((branchName) => branchName ? Promise.resolve( @@ -71,100 +60,21 @@ describe('workers/repository/process/limits', () => { { branchName: 'test' }, { branchName: null }, ] as never; - const res = await limits.getConcurrentPrsRemaining(config, branches); - expect(res).toBe(19); - }); - - it('returns MAX_SAFE_INTEGER if no concurrent limit', async () => { - config.prConcurrentLimit = 0; - const res = await limits.getConcurrentPrsRemaining(config, []); - expect(res).toBe(Number.MAX_SAFE_INTEGER); - }); - }); - - describe('getPrsRemaining()', () => { - it('returns hourly limit', async () => { - config.prHourlyLimit = 1; - platform.getPrList.mockResolvedValueOnce([]); - const res = await limits.getPrsRemaining(config, []); - expect(res).toBe(1); - }); - - it('returns concurrent limit', async () => { - config.prConcurrentLimit = 1; - const res = await limits.getPrsRemaining(config, []); + const res = await limits.getConcurrentPrsCount(config, branches); expect(res).toBe(1); }); }); - describe('getConcurrentBranchesRemaining()', () => { - it('calculates concurrent limit remaining', async () => { - config.branchConcurrentLimit = 20; - scm.branchExists.mockResolvedValueOnce(true); - const res = await limits.getConcurrentBranchesRemaining(config, [ - { branchName: 'foo' }, - ] as never); - expect(res).toBe(19); - }); - - it('defaults to prConcurrentLimit', async () => { - config.branchConcurrentLimit = null; - config.prConcurrentLimit = 20; - scm.branchExists.mockResolvedValueOnce(true); - const res = await limits.getConcurrentBranchesRemaining(config, [ + describe('getConcurrentBranchesCount()', () => { + it('calculates concurrent branches present', async () => { + scm.branchExists.mockImplementation((branchName) => + branchName ? Promise.resolve(true) : Promise.resolve(false), + ); + const res = await limits.getConcurrentBranchesCount([ { branchName: 'foo' }, + { branchName: null }, ] as never); - expect(res).toBe(19); - }); - - it('does not use prConcurrentLimit for explicit branchConcurrentLimit=0', async () => { - config.branchConcurrentLimit = 0; - config.prConcurrentLimit = 20; - const res = await limits.getConcurrentBranchesRemaining(config, []); - expect(res).toBe(Number.MAX_SAFE_INTEGER); - }); - - it('returns 10 if no limits are set', async () => { - const res = await limits.getConcurrentBranchesRemaining(config, []); - expect(res).toBe(10); - }); - - it('returns prConcurrentLimit if errored', async () => { - config.branchConcurrentLimit = 2; - // TODO: #22198 - const res = await limits.getConcurrentBranchesRemaining( - config, - null as never, - ); - expect(res).toBe(2); - }); - }); - - describe('getBranchesRemaining()', () => { - it('returns minimal of both limits', async () => { - platform.getPrList.mockResolvedValue([]); - - await expect( - limits.getBranchesRemaining( - { - ...config, - prHourlyLimit: 3, - branchConcurrentLimit: 5, - }, - [], - ), - ).resolves.toBe(3); - - await expect( - limits.getBranchesRemaining( - { - ...config, - prHourlyLimit: 11, - branchConcurrentLimit: 7, - }, - [], - ), - ).resolves.toBe(7); + expect(res).toBe(1); }); }); }); diff --git a/lib/workers/repository/process/limits.ts b/lib/workers/repository/process/limits.ts index 55aae025e095c89baed13fc25dec1a7b7e03ae5b..fb320f221f73213ea5381b331770f4764da0f152 100644 --- a/lib/workers/repository/process/limits.ts +++ b/lib/workers/repository/process/limits.ts @@ -1,141 +1,79 @@ import { DateTime } from 'luxon'; import type { RenovateConfig } from '../../../config/types'; import { logger } from '../../../logger'; -import type { Pr } from '../../../modules/platform'; import { platform } from '../../../modules/platform'; import { scm } from '../../../modules/platform/scm'; import { ExternalHostError } from '../../../types/errors/external-host-error'; import type { BranchConfig } from '../../types'; -export async function getPrHourlyRemaining( +export async function getPrHourlyCount( config: RenovateConfig, ): Promise<number> { - if (config.prHourlyLimit) { - try { - logger.debug('Calculating hourly PRs remaining'); - const prList = await platform.getPrList(); - const currentHourStart = DateTime.local().startOf('hour'); - logger.debug(`currentHourStart=${String(currentHourStart)}`); - const soFarThisHour = prList.filter( - (pr) => - pr.sourceBranch !== config.onboardingBranch && - pr.sourceBranch.startsWith(config.branchPrefix!) && - DateTime.fromISO(pr.createdAt!) > currentHourStart, - ); - const prsRemaining = Math.max( - 0, - config.prHourlyLimit - soFarThisHour.length, - ); - logger.debug(`PR hourly limit remaining: ${prsRemaining}`); - return prsRemaining; - } catch (err) { - // istanbul ignore if - if (err instanceof ExternalHostError) { - throw err; - } - logger.error({ err }, 'Error checking PRs created per hour'); - return config.prHourlyLimit; + try { + const prList = await platform.getPrList(); + const currentHourStart = DateTime.local().setZone('utc').startOf('hour'); + logger.debug( + `Calculating PRs created so far in this hour currentHourStart=${String(currentHourStart)}`, + ); + const soFarThisHour = prList.filter( + (pr) => + pr.sourceBranch !== config.onboardingBranch && + pr.sourceBranch.startsWith(config.branchPrefix!) && + DateTime.fromISO(pr.createdAt!) > currentHourStart, + ); + logger.debug( + `${soFarThisHour.length} PRs have been created so far in this hour.`, + ); + return soFarThisHour.length; + } catch (err) { + // istanbul ignore if + if (err instanceof ExternalHostError) { + throw err; } + logger.error({ err }, 'Error checking PRs created per hour'); + return 0; } - return Number.MAX_SAFE_INTEGER; } -export async function getConcurrentPrsRemaining( +export async function getConcurrentPrsCount( config: RenovateConfig, branches: BranchConfig[], ): Promise<number> { - if (config.prConcurrentLimit) { - logger.debug(`Calculating prConcurrentLimit (${config.prConcurrentLimit})`); + let openPrCount = 0; + for (const { branchName } of branches) { try { - const openPrs: Pr[] = []; - for (const { branchName } of branches) { - try { - const pr = await platform.getBranchPr(branchName, config.baseBranch); - if ( - pr && - pr.sourceBranch !== config.onboardingBranch && - pr.state === 'open' - ) { - openPrs.push(pr); - } - } catch (err) { - // istanbul ignore if - if (err instanceof ExternalHostError) { - throw err; - } else { - // no-op - } - } + const pr = await platform.getBranchPr(branchName, config.baseBranch); + if ( + pr && + pr.sourceBranch !== config.onboardingBranch && + pr.state === 'open' + ) { + openPrCount++; + } + } catch (err) { + // istanbul ignore if + if (err instanceof ExternalHostError) { + throw err; + } else { + // no-op } - logger.debug(`${openPrs.length} PRs are currently open`); - const concurrentRemaining = Math.max( - 0, - config.prConcurrentLimit - openPrs.length, - ); - logger.debug(`PR concurrent limit remaining: ${concurrentRemaining}`); - return concurrentRemaining; - } catch (err) /* istanbul ignore next */ { - logger.error({ err }, 'Error checking concurrent PRs'); - return config.prConcurrentLimit; } } - return Number.MAX_SAFE_INTEGER; -} -export async function getPrsRemaining( - config: RenovateConfig, - branches: BranchConfig[], -): Promise<number> { - const hourlyRemaining = await getPrHourlyRemaining(config); - const concurrentRemaining = await getConcurrentPrsRemaining(config, branches); - return Math.min(hourlyRemaining, concurrentRemaining); + logger.debug(`${openPrCount} PRs are currently open`); + return openPrCount; } -export async function getConcurrentBranchesRemaining( - config: RenovateConfig, +export async function getConcurrentBranchesCount( branches: BranchConfig[], ): Promise<number> { - const { branchConcurrentLimit, prConcurrentLimit } = config; - const limit = - typeof branchConcurrentLimit === 'number' - ? branchConcurrentLimit - : prConcurrentLimit; - if (typeof limit === 'number' && limit) { - logger.debug(`Calculating branchConcurrentLimit (${limit})`); - try { - const existingBranches: string[] = []; - for (const branch of branches) { - if (await scm.branchExists(branch.branchName)) { - existingBranches.push(branch.branchName); - } - } - - const existingCount = existingBranches.length; - logger.debug( - `${existingCount} already existing branches found: ${existingBranches.join()}`, - ); - - const concurrentRemaining = Math.max(0, limit - existingCount); - logger.debug(`Branch concurrent limit remaining: ${concurrentRemaining}`); - - return concurrentRemaining; - } catch (err) { - // TODO: #22198 should never throw - logger.error({ err }, 'Error checking concurrent branches'); - return limit; + let existingBranchCount = 0; + for (const branch of branches) { + if (await scm.branchExists(branch.branchName)) { + existingBranchCount++; } } - return Number.MAX_SAFE_INTEGER; -} -export async function getBranchesRemaining( - config: RenovateConfig, - branches: BranchConfig[], -): Promise<number> { - const hourlyRemaining = await getPrHourlyRemaining(config); - const concurrentRemaining = await getConcurrentBranchesRemaining( - config, - branches, - ); - return Math.min(hourlyRemaining, concurrentRemaining); + logger.debug(`${existingBranchCount} already existing branches found.`); + return existingBranchCount; } diff --git a/lib/workers/repository/process/write.spec.ts b/lib/workers/repository/process/write.spec.ts index f3a7405fabba465be6073b081b2c648ec21e9fdf..70a8165ffc5ff917ca6c9b825c1e9ef73473ff29 100644 --- a/lib/workers/repository/process/write.spec.ts +++ b/lib/workers/repository/process/write.spec.ts @@ -12,7 +12,7 @@ import type { } from '../../../util/cache/repository/types'; import { fingerprint } from '../../../util/fingerprint'; import type { LongCommitSha } from '../../../util/git/types'; -import { isLimitReached } from '../../global/limits'; +import { counts } from '../../global/limits'; import type { BranchConfig, BranchUpgradeConfig } from '../../types'; import * as _branchWorker from '../update/branch'; import * as _limits from './limits'; @@ -32,8 +32,9 @@ const repoCache = mocked(_repoCache); branchWorker.processBranch = jest.fn(); -limits.getPrsRemaining = jest.fn().mockResolvedValue(99); -limits.getBranchesRemaining = jest.fn().mockResolvedValue(99); +limits.getConcurrentPrsCount = jest.fn().mockResolvedValue(0); +limits.getConcurrentBranchesCount = jest.fn().mockResolvedValue(0); +limits.getPrHourlyCount = jest.fn().mockResolvedValue(0); let config: RenovateConfig; @@ -104,22 +105,35 @@ describe('workers/repository/process/write', () => { it('increments branch counter', async () => { const branchName = 'branchName'; - const branches: BranchConfig[] = [ - { baseBranch: 'main', branchName, upgrades: [], manager: 'npm' }, - { baseBranch: 'dev', branchName, upgrades: [], manager: 'npm' }, - ]; + const branches = partial<BranchConfig[]>([ + { + baseBranch: 'main', + branchName, + upgrades: partial<BranchUpgradeConfig>([{ prConcurrentLimit: 10 }]), + manager: 'npm', + }, + { + baseBranch: 'dev', + branchName, + upgrades: partial<BranchUpgradeConfig>([{ prConcurrentLimit: 10 }]), + manager: 'npm', + }, + ]); repoCache.getCache.mockReturnValueOnce({}); branchWorker.processBranch.mockResolvedValueOnce({ branchExists: true, result: 'pr-created', }); - scm.branchExists.mockResolvedValueOnce(false).mockResolvedValueOnce(true); - limits.getBranchesRemaining.mockResolvedValueOnce(1); - expect(isLimitReached('Branches')).toBeFalse(); + + limits.getConcurrentPrsCount.mockResolvedValue(0); + limits.getConcurrentBranchesCount.mockResolvedValue(0); + limits.getPrHourlyCount.mockResolvedValue(0); + + scm.branchExists.mockResolvedValueOnce(false).mockResolvedValue(true); GlobalConfig.set({ dryRun: 'full' }); config.baseBranches = ['main', 'dev']; await writeUpdates(config, branches); - expect(isLimitReached('Branches')).toBeTrue(); + expect(counts.get('Branches')).toBe(1); expect(addMeta).toHaveBeenCalledWith({ baseBranch: 'main', branch: branchName, diff --git a/lib/workers/repository/process/write.ts b/lib/workers/repository/process/write.ts index 4cc9f74c5d3adb8be8cdcd071c3c70223475fa26..6f7309b86b2f66000c6360f5f2237f843aca4a3f 100644 --- a/lib/workers/repository/process/write.ts +++ b/lib/workers/repository/process/write.ts @@ -7,11 +7,15 @@ import { getCache } from '../../../util/cache/repository'; import type { BranchCache } from '../../../util/cache/repository/types'; import { fingerprint } from '../../../util/fingerprint'; import { setBranchNewCommit } from '../../../util/git/set-branch-commit'; -import { incLimitedValue, setMaxLimit } from '../../global/limits'; +import { incCountValue, setCount } from '../../global/limits'; import type { BranchConfig, UpgradeFingerprintConfig } from '../../types'; import { processBranch } from '../update/branch'; import { upgradeFingerprintFields } from './fingerprint-fields'; -import { getBranchesRemaining, getPrsRemaining } from './limits'; +import { + getConcurrentBranchesCount, + getConcurrentPrsCount, + getPrHourlyCount, +} from './limits'; export type WriteUpdateResult = 'done' | 'automerged'; @@ -127,15 +131,15 @@ export async function writeUpdates( .sort() .join(', ')}`, ); - const prsRemaining = await getPrsRemaining(config, branches); - logger.debug(`Calculated maximum PRs remaining this run: ${prsRemaining}`); - setMaxLimit('PullRequests', prsRemaining); - const branchesRemaining = await getBranchesRemaining(config, branches); - logger.debug( - `Calculated maximum branches remaining this run: ${branchesRemaining}`, - ); - setMaxLimit('Branches', branchesRemaining); + const concurrentPrsCount = await getConcurrentPrsCount(config, branches); + setCount('ConcurrentPRs', concurrentPrsCount); + + const concurrentBranchesCount = await getConcurrentBranchesCount(branches); + setCount('Branches', concurrentBranchesCount); + + const prsThisHourCount = await getPrHourlyCount(config); + setCount('HourlyPRs', prsThisHourCount); for (const branch of branches) { const { baseBranch, branchName } = branch; @@ -182,7 +186,7 @@ export async function writeUpdates( return 'automerged'; } if (!branchExisted && (await scm.branchExists(branch.branchName))) { - incLimitedValue('Branches'); + incCountValue('Branches'); } } removeMeta(['branch', 'baseBranch']); diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index e76b4c2dce533d5ad47c15062868dc2aece72ea4..b6038b5ba9439954d2169046f96c97a9ce69c026 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -34,7 +34,7 @@ import { import { coerceNumber } from '../../../../util/number'; import { toMs } from '../../../../util/pretty-time'; import * as template from '../../../../util/template'; -import { isLimitReached } from '../../../global/limits'; +import { getCount, isLimitReached } from '../../../global/limits'; import type { BranchConfig, BranchResult, PrBlockedBy } from '../../../types'; import { embedChangelogs } from '../../changelog'; import { ensurePr, getPlatformPrOptions } from '../pr'; @@ -212,9 +212,14 @@ export async function processBranch( }; } } + + logger.debug( + `Open PR Count: ${getCount('ConcurrentPRs')}, Existing Branch Count: ${getCount('Branches')}, Hourly PR Count: ${getCount('HourlyPRs')}`, + ); + if ( !branchExists && - isLimitReached('Branches') && + isLimitReached('Branches', branchConfig) && !dependencyDashboardCheck && !config.isVulnerabilityAlert ) { diff --git a/lib/workers/repository/update/pr/index.spec.ts b/lib/workers/repository/update/pr/index.spec.ts index 2583501774f4b4ac0d12771f149d58e5820b4ee2..20de0385e341a8fa80ea49ed16f4d86c16f34f6f 100644 --- a/lib/workers/repository/update/pr/index.spec.ts +++ b/lib/workers/repository/update/pr/index.spec.ts @@ -89,8 +89,9 @@ describe('workers/repository/update/pr/index', () => { const res = await ensurePr(config); expect(res).toEqual({ type: 'with-pr', pr }); - expect(limits.incLimitedValue).toHaveBeenCalledOnce(); - expect(limits.incLimitedValue).toHaveBeenCalledWith('PullRequests'); + expect(limits.incCountValue).toHaveBeenCalledTimes(2); + expect(limits.incCountValue).toHaveBeenCalledWith('ConcurrentPRs'); + expect(limits.incCountValue).toHaveBeenCalledWith('HourlyPRs'); expect(logger.logger.info).toHaveBeenCalledWith( { pr: pr.number, prTitle }, 'PR created', diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index aa6817daf31a3517fd7e9a9ae7fed96afc42bbf7..6b1a05a75e0af9a40dbc021c402b64b0a15d7648 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -27,7 +27,7 @@ import { stripEmojis } from '../../../../util/emoji'; import { fingerprint } from '../../../../util/fingerprint'; import { getBranchLastCommitTime } from '../../../../util/git'; import { memoize } from '../../../../util/memoize'; -import { incLimitedValue, isLimitReached } from '../../../global/limits'; +import { incCountValue, isLimitReached } from '../../../global/limits'; import type { BranchConfig, BranchUpgradeConfig, @@ -482,7 +482,7 @@ export async function ensurePr( try { if ( !dependencyDashboardCheck && - isLimitReached('PullRequests') && + isLimitReached('ConcurrentPRs', prConfig) && !config.isVulnerabilityAlert ) { logger.debug('Skipping PR - limit reached'); @@ -499,7 +499,8 @@ export async function ensurePr( milestone: config.milestone, }); - incLimitedValue('PullRequests'); + incCountValue('ConcurrentPRs'); + incCountValue('HourlyPRs'); logger.info({ pr: pr?.number, prTitle }, 'PR created'); } catch (err) { logger.debug({ err }, 'Pull request creation error');