Skip to content

Commit a18df2d

Browse files
doclouloumcowger
andauthored
refactor(config-service): update quota scheduler reload logic and add tests (#414)
- Changed the condition for reloading the quota scheduler to check if it is initialized instead of checking for registered checkers. - Added a new `isInitialized` method to the `QuotaScheduler` class to encapsulate the initialization state. - Updated tests for `ConfigService` to verify the reload behavior when the scheduler is initialized. - Enhanced `QuotaScheduler` tests to ensure proper handling of checker updates and rescheduling on reload. _When Plexus starts with no quota checkers configured, an admin can later add the first provider quota checker from the UI and have it take effect immediately, without restarting the server. Previously, the scheduler reload was skipped because there were no existing checker IDs, so the new checker was saved but not registered or scheduled._ Co-authored-by: Matt Cowger <mcowger@users.noreply.github.com>
1 parent 8ac523e commit a18df2d

4 files changed

Lines changed: 143 additions & 26 deletions

File tree

packages/backend/src/services/__tests__/config-service.test.ts

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
2+
import type { ProviderConfig } from '../../config';
3+
4+
const mockScheduler = vi.hoisted(() => ({
5+
getCheckerIds: vi.fn(() => []),
6+
isInitialized: vi.fn(() => true),
7+
reload: vi.fn(() => Promise.resolve()),
8+
}));
29

310
vi.mock('../quota/quota-scheduler', () => ({
411
QuotaScheduler: {
5-
getInstance: vi.fn(() => ({
6-
getCheckerIds: vi.fn(() => []),
7-
reload: vi.fn(),
8-
})),
12+
getInstance: vi.fn(() => mockScheduler),
913
},
1014
}));
1115

@@ -57,6 +61,9 @@ describe('ConfigService write coalescing', () => {
5761
ConfigService.resetInstance();
5862
rebuildCount = 0;
5963
mockRepo = createMockRepo();
64+
mockScheduler.getCheckerIds.mockReturnValue([]);
65+
mockScheduler.isInitialized.mockReturnValue(true);
66+
mockScheduler.reload.mockClear();
6067

6168
service = new ConfigService(mockRepo as any);
6269

@@ -120,4 +127,30 @@ describe('ConfigService write coalescing', () => {
120127

121128
vi.useRealTimers();
122129
});
130+
131+
it('reloads quota scheduler after first quota checker is saved when scheduler is initialized', async () => {
132+
vi.useFakeTimers();
133+
134+
const providerConfig: ProviderConfig = {
135+
api_base_url: 'https://api.synthetic.new',
136+
api_key: 'synthetic-key',
137+
disable_cooldown: false,
138+
estimateTokens: false,
139+
useClaudeMasking: false,
140+
quota_checker: {
141+
type: 'synthetic',
142+
enabled: true,
143+
intervalMinutes: 60,
144+
options: {},
145+
},
146+
};
147+
148+
await service.saveProvider('synthetic-provider', providerConfig);
149+
await service.flush();
150+
151+
expect(mockScheduler.isInitialized).toHaveBeenCalled();
152+
expect(mockScheduler.reload).toHaveBeenCalled();
153+
154+
vi.useRealTimers();
155+
});
123156
});

packages/backend/src/services/config-service.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,10 +408,10 @@ export class ConfigService {
408408

409409
// Reload the quota scheduler with the updated quota configs so that
410410
// changes saved via the UI take effect without a restart.
411-
// Only reload if the scheduler has already been initialized (has checkers registered);
411+
// Only reload if the scheduler has already been initialized;
412412
// on startup, index.ts calls quotaScheduler.initialize() explicitly after this.
413413
const scheduler = QuotaScheduler.getInstance();
414-
if (scheduler.getCheckerIds().length > 0) {
414+
if (scheduler.isInitialized()) {
415415
scheduler.reload(quotas).catch((err) => {
416416
logger.warn(`Failed to reload QuotaScheduler after config change: ${err}`);
417417
});

packages/backend/src/services/quota/__tests__/quota-scheduler.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { QuotaScheduler } from '../quota-scheduler';
1212
import { CooldownManager } from '../../cooldown-manager';
1313
import type { MeterCheckResult, Meter } from '../../../types/meter';
1414
import type { QuotaConfig } from '../../../config';
15+
import { registerSpy } from '../../../../test/test-utils';
1516

1617
const CHECKER_ID = 'quota-persistence-checker';
1718

@@ -153,6 +154,60 @@ describe('QuotaScheduler persistence', () => {
153154
expect(rows[0]?.utilizationState).toBe('reported');
154155
expect(rows[0]?.utilizationPercent).toBeCloseTo(15);
155156
});
157+
158+
it('marks scheduler initialized when initialize receives no quota configs', async () => {
159+
const scheduler = QuotaScheduler.getInstance();
160+
161+
await scheduler.initialize([]);
162+
163+
expect(scheduler.isInitialized()).toBe(true);
164+
expect(scheduler.getCheckerIds()).toEqual([]);
165+
});
166+
167+
it('updates existing checker options and reschedules interval changes on reload', async () => {
168+
vi.useFakeTimers();
169+
170+
try {
171+
const scheduler = QuotaScheduler.getInstance();
172+
const runCheckNow = registerSpy(scheduler, 'runCheckNow').mockResolvedValue(null);
173+
const initialConfig: QuotaConfig = {
174+
id: 'synthetic-reload-checker',
175+
provider: 'synthetic-provider',
176+
type: 'synthetic',
177+
enabled: true,
178+
intervalMinutes: 60,
179+
options: {
180+
apiKey: 'synthetic-key',
181+
endpoint: 'https://old.example.com/v2/quotas',
182+
},
183+
};
184+
185+
await scheduler.initialize([initialConfig]);
186+
runCheckNow.mockClear();
187+
188+
await scheduler.reload([
189+
{
190+
...initialConfig,
191+
intervalMinutes: 1,
192+
options: {
193+
apiKey: 'synthetic-key',
194+
endpoint: 'https://new.example.com/v2/quotas',
195+
},
196+
},
197+
]);
198+
199+
const configs = Reflect.get(scheduler, 'configs') as Map<string, QuotaConfig>;
200+
const updatedConfig = configs.get('synthetic-reload-checker');
201+
expect(updatedConfig?.intervalMinutes).toBe(1);
202+
expect(updatedConfig?.options.endpoint).toBe('https://new.example.com/v2/quotas');
203+
204+
await vi.advanceTimersByTimeAsync(60_000);
205+
206+
expect(runCheckNow).toHaveBeenCalledWith('synthetic-reload-checker');
207+
} finally {
208+
vi.useRealTimers();
209+
}
210+
});
156211
});
157212

158213
describe('QuotaScheduler maxUtilizationPercent', () => {

packages/backend/src/services/quota/quota-scheduler.ts

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,10 @@ export class QuotaScheduler {
272272
return Array.from(this.configs.keys());
273273
}
274274

275+
isInitialized(): boolean {
276+
return this.checkersLoaded;
277+
}
278+
275279
async getLatestQuota(checkerId: string): Promise<MeterCheckResult | null> {
276280
try {
277281
const { db, schema } = this.ensureDb();
@@ -397,39 +401,64 @@ export class QuotaScheduler {
397401
}
398402

399403
const existingIds = new Set(this.configs.keys());
400-
const newConfigs = quotaConfigs.filter((c) => !existingIds.has(c.id) && c.enabled);
404+
const activeConfigs = quotaConfigs.filter((c) => c.enabled);
405+
const activeIds = new Set(activeConfigs.map((c) => c.id));
401406

402-
for (const config of newConfigs) {
407+
for (const id of existingIds) {
408+
if (!activeIds.has(id)) {
409+
const intervalId = this.intervals.get(id);
410+
if (intervalId) {
411+
clearInterval(intervalId);
412+
this.intervals.delete(id);
413+
}
414+
this.configs.delete(id);
415+
logger.info(`Removed quota checker '${id}' on reload`);
416+
}
417+
}
418+
419+
for (const config of activeConfigs) {
403420
if (!getCheckerDefinition(config.type)) {
404421
logger.error(`Unknown quota checker type '${config.type}' for checker '${config.id}'`);
405422
continue;
406423
}
424+
425+
const existingConfig = this.configs.get(config.id);
426+
const intervalChanged = existingConfig?.intervalMinutes !== config.intervalMinutes;
427+
407428
this.configs.set(config.id, config);
429+
430+
if (existingConfig && !intervalChanged) {
431+
logger.debug(`Updated quota checker '${config.id}' on reload`);
432+
continue;
433+
}
434+
435+
if (existingConfig && intervalChanged) {
436+
const intervalId = this.intervals.get(config.id);
437+
if (intervalId) clearInterval(intervalId);
438+
this.intervals.delete(config.id);
439+
logger.info(
440+
`Rescheduled quota checker '${config.id}' from ${existingConfig.intervalMinutes} to ${config.intervalMinutes} minutes`
441+
);
442+
} else {
443+
logger.info(
444+
`Registered quota checker '${config.id}' (${config.type}) for provider '${config.provider}'`
445+
);
446+
}
447+
448+
if (this.intervals.has(config.id)) continue;
449+
408450
logger.info(
409-
`Registered quota checker '${config.id}' (${config.type}) for provider '${config.provider}'`
451+
`Scheduled quota checker '${config.id}' to run every ${config.intervalMinutes} minutes`
410452
);
411453

412454
const intervalMs = config.intervalMinutes * 60 * 1000;
413455
const intervalId = setInterval(() => this.runCheckNow(config.id), intervalMs);
414456
this.intervals.set(config.id, intervalId);
415-
logger.info(
416-
`Scheduled quota checker '${config.id}' to run every ${config.intervalMinutes} minutes`
417-
);
418-
this.runCheckNow(config.id).catch((error) => {
419-
logger.error(`Initial quota check failed for '${config.id}' on reload: ${error}`);
420-
});
421-
}
422457

423-
const loadedIds = new Set(quotaConfigs.filter((c) => c.enabled).map((c) => c.id));
424-
for (const id of existingIds) {
425-
if (!loadedIds.has(id)) {
426-
const intervalId = this.intervals.get(id);
427-
if (intervalId) {
428-
clearInterval(intervalId);
429-
this.intervals.delete(id);
430-
}
431-
this.configs.delete(id);
432-
logger.info(`Removed quota checker '${id}' on reload`);
458+
if (!existingConfig) {
459+
this.runCheckNow(config.id).catch((error) => {
460+
logger.error(`Initial quota check failed for '${config.id}' on reload: ${error}`);
461+
});
433462
}
434463
}
435464
}

0 commit comments

Comments
 (0)