From f93050bb4939b8beaac45b5dc1533c02ae1c8d81 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Sat, 10 Oct 2026 15:08:31 -0400 Subject: [PATCH] fix(alarm): forget idle alarms and cap how many the client holds AlarmClient kept every alarm id it had seen for the life of the process, and ids that embed a user, key or invoice grew it without bound. The registry is now ordered by last occurrence: alarms idle for 24 hours are dropped on the next create, and past 1000 the least recently raised goes first. Clearing or evicting an alarm only removes its short-id alias when the alias still points at it. --- doc/alarms.md | 5 + src/backend/clients/alarm/AlarmClient.test.ts | 125 +++++++++++++++++- src/backend/clients/alarm/AlarmClient.ts | 59 +++++++-- 3 files changed, 178 insertions(+), 11 deletions(-) diff --git a/doc/alarms.md b/doc/alarms.md index f8f23a284..658f77d15 100644 --- a/doc/alarms.md +++ b/doc/alarms.md @@ -134,6 +134,11 @@ occurrence is reported up to a small burst, then one per interval. A fault recurring hundreds of times a second still counts every occurrence, but it does not write a log line or build an alert payload for each one. +The count lasts only as long as the client remembers the alarm. One that +hasn't recurred for 24 hours is forgotten, and the client holds at most 1000 +at once, dropping the least recently raised first. Either way the next +occurrence is a new alarm, with its count and burst starting over. + Below that, the chat transport won't repost the same alarm id within `repeatThrottleMs` (default 15 minutes). The first occurrence always posts, and the next one that gets through reports how many piled up in between — diff --git a/src/backend/clients/alarm/AlarmClient.test.ts b/src/backend/clients/alarm/AlarmClient.test.ts index 75c92272b..f6c4b9bee 100644 --- a/src/backend/clients/alarm/AlarmClient.test.ts +++ b/src/backend/clients/alarm/AlarmClient.test.ts @@ -1,5 +1,9 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { AlarmClient } from './AlarmClient'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + ALARM_IDLE_TTL_MS, + AlarmClient, + MAX_TRACKED_ALARMS, +} from './AlarmClient'; import type { IConfig, IPagerConfig } from '../../types'; import type { AlertPayload } from './types'; @@ -342,6 +346,19 @@ describe('AlarmClient alarm registry', () => { expect(client.get(shortId)).toBeUndefined(); }); + it('keeps the short id of another alarm that shares it when one is cleared', () => { + const client = makeClient(); + // These two ids hash to the same short id. + client.create('alarm:274', 'first'); + const shortId = client.get('alarm:274')!.shortId; + client.create('alarm:643', 'second'); + expect(client.get('alarm:643')!.shortId).toBe(shortId); + + client.clear('alarm:274'); + + expect(client.get(shortId)?.id).toBe('alarm:643'); + }); + it('ignores a clear for an alarm that is not active', () => { const client = makeClient(); expect(() => client.clear('never-raised')).not.toThrow(); @@ -428,6 +445,110 @@ describe('AlarmClient alarm registry', () => { }); }); +describe('AlarmClient alarm eviction', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('forgets alarms that stay quiet past the idle window, under both ids', () => { + const client = makeClient(); + const ids = Array.from( + { length: 50 }, + (_, i) => `metering_usage_dropped:user-${i}`, + ); + for (const id of ids) client.create(id, 'usage dropped'); + const shortIds = ids.map((id) => client.get(id)!.shortId); + + vi.advanceTimersByTime(ALARM_IDLE_TTL_MS + 1); + client.create('unrelated', 'the next alarm sweeps the registry'); + + expect(ids.filter((id) => client.get(id))).toEqual([]); + expect(shortIds.filter((shortId) => client.get(shortId))).toEqual([]); + expect(client.get('unrelated')).toBeDefined(); + }); + + it('treats a recurrence after the idle window as a new alarm', () => { + const client = makeClient(); + const seen = capture(client); + + client.create('scan:failed', 'first'); + client.create('scan:failed', 'second'); + vi.advanceTimersByTime(ALARM_IDLE_TTL_MS + 1); + client.create('scan:failed', 'back again'); + + expect(seen.at(-1)).toMatchObject({ repeatCount: 1, isRepeat: false }); + // A fresh start keeps per-occurrence incident keys from colliding. + expect(seen.at(-1)!.dedupKey).not.toBe(seen[0].dedupKey); + }); + + it('still treats a repeat inside the window as a repeat after a sweep', () => { + const client = makeClient(); + const seen = capture(client); + + for (let i = 0; i < 10; i++) client.create('flap', 'same fault'); + vi.advanceTimersByTime(ALARM_IDLE_TTL_MS - 1000); + client.create('other', 'sweeps the registry'); + client.create('flap', 'same fault'); + + // Occurrence 11 is past the burst, so nothing new goes out for it. + expect(seen.map((alert) => alert.id)).toEqual([ + ...Array(10).fill('flap'), + 'other', + ]); + expect(client.get('flap')?.count).toBe(11); + }); + + it('keeps an alarm that keeps recurring for longer than the window', () => { + const client = makeClient(); + + for (let i = 0; i < 6; i++) { + client.create('hot', 'still broken'); + vi.advanceTimersByTime(ALARM_IDLE_TTL_MS / 2); + client.create(`noise:${i}`, 'sweeps the registry'); + } + + expect(client.get('hot')?.count).toBe(6); + }); + + it('drops the least recently raised alarm once full', () => { + const client = makeClient(); + + for (let i = 0; i < MAX_TRACKED_ALARMS; i++) { + client.create(`user:${i}`, 'per-user alarm'); + vi.advanceTimersByTime(1); + } + // Raised again, so user:1 is now the one seen longest ago. + client.create('user:0', 'per-user alarm'); + client.create('one-more', 'over the cap'); + + expect(client.get('user:1')).toBeUndefined(); + expect(client.get('user:0')?.count).toBe(2); + expect(client.get('user:2')).toBeDefined(); + expect(client.get('one-more')).toBeDefined(); + }); + + it('never holds more than the cap', () => { + const client = makeClient(); + const ids = Array.from( + { length: MAX_TRACKED_ALARMS * 3 }, + (_, i) => `user:${i}`, + ); + + for (const id of ids) client.create(id, 'per-user alarm'); + + const held = ids.filter((id) => client.get(id)); + expect(held).toHaveLength(MAX_TRACKED_ALARMS); + expect(held[0]).toBe(`user:${MAX_TRACKED_ALARMS * 2}`); + }); +}); + describe('AlarmClient known-error rules', () => { beforeEach(() => { vi.spyOn(console, 'error').mockImplementation(() => {}); diff --git a/src/backend/clients/alarm/AlarmClient.ts b/src/backend/clients/alarm/AlarmClient.ts index 7b4f949c0..e968ced7d 100644 --- a/src/backend/clients/alarm/AlarmClient.ts +++ b/src/backend/clients/alarm/AlarmClient.ts @@ -61,14 +61,24 @@ interface RegisteredHandler { /** Severity used when neither the call site nor config picks one. */ const FALLBACK_SEVERITY: PagerSeverity = 'critical'; /** - * How many recent occurrences an alarm keeps. An alarm is never cleared unless - * something calls `clear`, and a hot one repeats for as long as the fault lasts - * — so retaining every occurrence means retaining every message and field set - * it was ever raised with, request bodies and actors included, for the life of - * the process. The last few are what a human reads; the rest is only a count, - * and `count` keeps that. + * How many recent occurrences an alarm keeps. A hot alarm repeats for as long + * as the fault lasts, so retaining every occurrence means retaining every + * message and field set it was ever raised with, request bodies and actors + * included. The last few are what a human reads; the rest is only a count, and + * `count` keeps that. */ const OCCURRENCE_HISTORY_LIMIT = 20; +/** + * An alarm that hasn't recurred for this long is forgotten; its next occurrence + * is a new alarm with a fresh count. Kept far past the chat transport's repeat + * throttle so a slowly recurring fault still counts as one alarm. + */ +export const ALARM_IDLE_TTL_MS = 24 * 60 * 60 * 1000; +/** + * Most alarms held at once. Ids that embed a user or key can outrun the idle + * window; past this the least recently raised alarm is dropped. + */ +export const MAX_TRACKED_ALARMS = 1000; // A fast-recurring fault repeats far faster than it is useful to report, and // every report allocates a log line and a payload. Report every occurrence up @@ -187,6 +197,7 @@ function cleanFields(fields: AlarmFields): Record { * `pager.severityOverrides` in {@link IConfig}. */ export class AlarmClient extends PuterClient { + /** Ordered by last occurrence: a repeat moves its alarm to the back. */ private alarms = new Map(); private aliases = new Map(); private alertHandlers: RegisteredHandler[] = []; @@ -290,7 +301,8 @@ export class AlarmClient extends PuterClient { /** * Create or update an alarm. If the alarm ID already exists, the occurrence - * count is incremented and a repeat alert is dispatched. + * count is incremented and a repeat alert is dispatched. An alarm idle for + * {@link ALARM_IDLE_TTL_MS} is forgotten, so its next occurrence is new. * * `severity` decides where the alarm lands: * @@ -321,9 +333,12 @@ export class AlarmClient extends PuterClient { return; } + this.evict(); const existing = this.alarms.get(id); if (existing) { + this.alarms.delete(id); + this.alarms.set(id, existing); this.recordOccurrence(existing, message, fields); this.handleRepeat(existing); return; @@ -348,6 +363,7 @@ export class AlarmClient extends PuterClient { this.alarms.set(id, alarm); this.aliases.set(alarm.shortId, alarm); this.recordOccurrence(alarm, message, fields); + this.evict(); this.handleNew(alarm); } @@ -356,8 +372,7 @@ export class AlarmClient extends PuterClient { const alarm = this.alarms.get(id); if (!alarm) return; - this.alarms.delete(id); - this.aliases.delete(alarm.shortId); + this.forget(alarm); console.log(`[alarm] CLEAR ${displayId(alarm)} :: ${alarm.message}`); } @@ -395,6 +410,32 @@ export class AlarmClient extends PuterClient { // -- Internals ---------------------------------------------------- + /** + * Drop idle alarms, then the least recently raised ones past the cap. The + * map's order puts both at the front, so this stops at the first keeper. + */ + private evict(): void { + const now = Date.now(); + for (const alarm of this.alarms.values()) { + const lastSeen = alarm.timestamps.at(-1) ?? alarm.started; + if ( + this.alarms.size <= MAX_TRACKED_ALARMS && + now - lastSeen < ALARM_IDLE_TTL_MS + ) { + return; + } + this.forget(alarm); + } + } + + private forget(alarm: Alarm): void { + this.alarms.delete(alarm.id); + // Short ids can collide; leave an alias that now names another alarm. + if (this.aliases.get(alarm.shortId) === alarm) { + this.aliases.delete(alarm.shortId); + } + } + private recordOccurrence( alarm: Alarm, message: string,