From b3930b31f249a48ab61f8b022062d1f6bd377947 Mon Sep 17 00:00:00 2001 From: Alex Holliday Date: Mon, 13 Apr 2026 11:39:29 -0700 Subject: [PATCH 1/2] remove in memory modification of monitor stats --- server/src/db/models/MonitorStats.ts | 2 +- .../monitor-stats/IMonitorStatsRepository.ts | 7 +- .../MongoMonitorStatsRepository.ts | 46 +++++- .../TimescaleMonitorStatsRepository.ts | 81 ++++++---- .../service/infrastructure/statusService.ts | 106 ++----------- server/src/types/monitorStats.ts | 6 + .../test/unit/services/statusService.test.ts | 147 ++++-------------- 7 files changed, 147 insertions(+), 248 deletions(-) diff --git a/server/src/db/models/MonitorStats.ts b/server/src/db/models/MonitorStats.ts index 5780de476..4ef2dab61 100755 --- a/server/src/db/models/MonitorStats.ts +++ b/server/src/db/models/MonitorStats.ts @@ -16,7 +16,7 @@ const MonitorStatsSchema = new Schema( monitorId: { type: Schema.Types.ObjectId, ref: "Monitor", - index: true, + unique: true, required: true, }, avgResponseTime: { diff --git a/server/src/repositories/monitor-stats/IMonitorStatsRepository.ts b/server/src/repositories/monitor-stats/IMonitorStatsRepository.ts index 4abdeb33f..101283e32 100644 --- a/server/src/repositories/monitor-stats/IMonitorStatsRepository.ts +++ b/server/src/repositories/monitor-stats/IMonitorStatsRepository.ts @@ -1,11 +1,12 @@ -import type { MonitorStats } from "@/types/index.js"; +import type { MonitorStats, CheckResultInput } from "@/types/index.js"; + export interface IMonitorStatsRepository { // create create(data: Omit): Promise; // single fetch findByMonitorId(monitorId: string): Promise; - // update - updateByMonitorId(monitorId: string, data: Omit): Promise; + // Upsert must be atomic to prevent race conditions + updateByMonitorId(monitorId: string, result: CheckResultInput): Promise; // delete deleteByMonitorId(monitorId: string): Promise; deleteByMonitorIds(monitorIds: string[]): Promise; diff --git a/server/src/repositories/monitor-stats/MongoMonitorStatsRepository.ts b/server/src/repositories/monitor-stats/MongoMonitorStatsRepository.ts index 774405587..ea255f7ad 100644 --- a/server/src/repositories/monitor-stats/MongoMonitorStatsRepository.ts +++ b/server/src/repositories/monitor-stats/MongoMonitorStatsRepository.ts @@ -1,5 +1,5 @@ import { type MonitorStatsDocument, MonitorStatsModel } from "@/db/models/index.js"; -import type { MonitorStats } from "@/types/index.js"; +import type { MonitorStats, CheckResultInput } from "@/types/index.js"; import { IMonitorStatsRepository } from "@/repositories/index.js"; import mongoose from "mongoose"; import { AppError } from "@/utils/AppError.js"; @@ -46,10 +46,48 @@ class MongoMonitorStatsRepository implements IMonitorStatsRepository { return this.toEntity(monitorStats); }; - updateByMonitorId = async (monitorId: string, data: Omit): Promise => { - const updated = await MonitorStatsModel.findOneAndUpdate({ monitorId: new mongoose.Types.ObjectId(monitorId) }, { $set: data }, { new: true }); + updateByMonitorId = async (monitorId: string, result: CheckResultInput): Promise => { + const { status, responseTime, now } = result; + const objectId = new mongoose.Types.ObjectId(monitorId); + const upDelta = status ? 1 : 0; + const downDelta = status ? 0 : 1; + const pipeline = [ + { + $set: { + monitorId: objectId, + totalChecks: { $add: [{ $ifNull: ["$totalChecks", 0] }, 1] }, + totalUpChecks: { $add: [{ $ifNull: ["$totalUpChecks", 0] }, upDelta] }, + totalDownChecks: { $add: [{ $ifNull: ["$totalDownChecks", 0] }, downDelta] }, + avgResponseTime: { + $divide: [ + { + $add: [{ $multiply: [{ $ifNull: ["$avgResponseTime", 0] }, { $ifNull: ["$totalChecks", 0] }] }, responseTime], + }, + { $add: [{ $ifNull: ["$totalChecks", 0] }, 1] }, + ], + }, + uptimePercentage: { + $divide: [{ $add: [{ $ifNull: ["$totalUpChecks", 0] }, upDelta] }, { $add: [{ $ifNull: ["$totalChecks", 0] }, 1] }], + }, + maxResponseTime: { $max: [{ $ifNull: ["$maxResponseTime", 0] }, responseTime] }, + lastResponseTime: responseTime, + lastCheckTimestamp: now, + timeOfLastFailure: status + ? { + $cond: [{ $eq: [{ $ifNull: ["$timeOfLastFailure", 0] }, 0] }, now, "$timeOfLastFailure"], + } + : 0, + }, + }, + ]; + + const updated = await MonitorStatsModel.findOneAndUpdate({ monitorId: objectId }, pipeline, { + upsert: true, + new: true, + setDefaultsOnInsert: true, + }); if (!updated) { - throw new AppError({ message: "Monitor stats not found", status: 404 }); + throw new AppError({ message: "Failed to apply check result to monitor stats", status: 500 }); } return this.toEntity(updated); }; diff --git a/server/src/repositories/monitor-stats/TimescaleMonitorStatsRepository.ts b/server/src/repositories/monitor-stats/TimescaleMonitorStatsRepository.ts index f5801a1c0..308dc1b64 100644 --- a/server/src/repositories/monitor-stats/TimescaleMonitorStatsRepository.ts +++ b/server/src/repositories/monitor-stats/TimescaleMonitorStatsRepository.ts @@ -1,6 +1,6 @@ import type { Pool } from "pg"; import type { IMonitorStatsRepository } from "./IMonitorStatsRepository.js"; -import type { MonitorStats } from "@/types/monitorStats.js"; +import type { MonitorStats, CheckResultInput } from "@/types/monitorStats.js"; import { AppError } from "@/utils/AppError.js"; interface MonitorStatsRow { @@ -61,37 +61,60 @@ export class TimescaleMonitorStatsRepository implements IMonitorStatsRepository return this.toEntity(row); }; - updateByMonitorId = async (monitorId: string, data: Omit): Promise => { - const result = await this.pool.query( - `UPDATE monitor_stats SET - avg_response_time = $2, - max_response_time = $3, - total_checks = $4, - total_up_checks = $5, - total_down_checks = $6, - uptime_percentage = $7, - last_check_timestamp = $8, - last_response_time = $9, - time_of_last_failure = $10, + updateByMonitorId = async (monitorId: string, result: CheckResultInput): Promise => { + const { status, responseTime, now } = result; + const upDelta = status ? 1 : 0; + const downDelta = status ? 0 : 1; + const nowTs = new Date(now); + + // Atomic upsert via INSERT ... ON CONFLICT DO UPDATE + const queryResult = await this.pool.query( + `INSERT INTO monitor_stats ( + monitor_id, + avg_response_time, + max_response_time, + total_checks, + total_up_checks, + total_down_checks, + uptime_percentage, + last_check_timestamp, + last_response_time, + time_of_last_failure + ) VALUES ( + $1, + $2, + $2, + 1, + $3, + $4, + $3::float, + $5, + $2, + CASE WHEN $6::boolean THEN $5 ELSE NULL END + ) + ON CONFLICT (monitor_id) DO UPDATE SET + total_checks = monitor_stats.total_checks + 1, + total_up_checks = monitor_stats.total_up_checks + $3, + total_down_checks = monitor_stats.total_down_checks + $4, + avg_response_time = (monitor_stats.avg_response_time * monitor_stats.total_checks + $2) + / (monitor_stats.total_checks + 1), + uptime_percentage = (monitor_stats.total_up_checks + $3)::float + / (monitor_stats.total_checks + 1), + max_response_time = GREATEST(monitor_stats.max_response_time, $2), + last_response_time = $2, + last_check_timestamp = $5, + time_of_last_failure = CASE + WHEN $6::boolean AND monitor_stats.time_of_last_failure IS NULL THEN $5 + WHEN $6::boolean THEN monitor_stats.time_of_last_failure + ELSE NULL + END, updated_at = NOW() - WHERE monitor_id = $1 - RETURNING ${COLUMNS}`, - [ - monitorId, - data.avgResponseTime, - data.maxResponseTime, - data.totalChecks, - data.totalUpChecks, - data.totalDownChecks, - data.uptimePercentage, - data.lastCheckTimestamp ? new Date(data.lastCheckTimestamp) : null, - data.lastResponseTime, - data.timeOfLastFailure ? new Date(data.timeOfLastFailure) : null, - ] + RETURNING ${COLUMNS}`, + [monitorId, responseTime, upDelta, downDelta, nowTs, status] ); - const row = result.rows[0]; + const row = queryResult.rows[0]; if (!row) { - throw new AppError({ message: "Monitor stats not found", status: 404 }); + throw new AppError({ message: "Failed to apply check result to monitor stats", status: 500 }); } return this.toEntity(row); }; diff --git a/server/src/service/infrastructure/statusService.ts b/server/src/service/infrastructure/statusService.ts index 9826d6603..4b591694d 100755 --- a/server/src/service/infrastructure/statusService.ts +++ b/server/src/service/infrastructure/statusService.ts @@ -14,7 +14,6 @@ import type { GameStatusPayload, GrpcStatusPayload, CheckSnapshot, - MonitorStats, CheckDiskInfo, } from "@/types/index.js"; import { AppError } from "@/utils/AppError.js"; @@ -68,97 +67,11 @@ export class StatusService implements IStatusService { async updateRunningStats(monitor: Monitor, networkResponse: MonitorStatusResponse) { try { - const monitorId = monitor.id; - const { responseTime, status } = networkResponse; - let existingStats: MonitorStats | null = null; - existingStats = await this.monitorStatsRepository - .findByMonitorId(monitorId) - .then((result) => result) - .catch(() => { - this.logger.debug({ - service: SERVICE_NAME, - method: "updateRunningStats", - message: `No existing stats found for monitor ${monitorId}, initializing new stats.`, - }); - return null; - }); - - let stats: Omit; - - if (!existingStats) { - // Initialize new stats - stats = { - avgResponseTime: 0, - maxResponseTime: 0, - totalChecks: 0, - totalUpChecks: 0, - totalDownChecks: 0, - uptimePercentage: 0, - lastResponseTime: 0, - lastCheckTimestamp: 0, - }; - } else { - // Use existing stats (omit id, monitorId, createdAt, updatedAt) - stats = { - avgResponseTime: existingStats.avgResponseTime, - maxResponseTime: existingStats.maxResponseTime, - totalChecks: existingStats.totalChecks, - totalUpChecks: existingStats.totalUpChecks, - totalDownChecks: existingStats.totalDownChecks, - uptimePercentage: existingStats.uptimePercentage, - lastResponseTime: existingStats.lastResponseTime, - lastCheckTimestamp: existingStats.lastCheckTimestamp, - timeOfLastFailure: existingStats.timeOfLastFailure, - }; - } - - // Update stats - stats.totalChecks++; - - // Last response time - stats.lastResponseTime = responseTime ?? 0; - - // Max response time - if (responseTime && responseTime > stats.maxResponseTime) { - stats.maxResponseTime = responseTime; - } - - // Avg response time: - let avgResponseTime = stats.avgResponseTime; - if (typeof responseTime !== "undefined" && responseTime !== null) { - if (avgResponseTime === 0) { - avgResponseTime = responseTime; - } else { - avgResponseTime = (avgResponseTime * (stats.totalChecks - 1) + responseTime) / stats.totalChecks; - } - } - stats.avgResponseTime = avgResponseTime; - - // Total checks - if (status === true) { - stats.totalUpChecks++; - // Update the timeSinceLastFailure if needed - if (stats.timeOfLastFailure === 0) { - stats.timeOfLastFailure = new Date().getTime(); - } - } else { - stats.totalDownChecks++; - stats.timeOfLastFailure = 0; - } - - // Calculate uptime percentage - stats.uptimePercentage = stats.totalUpChecks / stats.totalChecks; - - // latest check - stats.lastCheckTimestamp = new Date().getTime(); - - // Create or update - if (!existingStats) { - await this.monitorStatsRepository.create({ monitorId, ...stats }); - } else { - await this.monitorStatsRepository.updateByMonitorId(monitorId, stats); - } - + await this.monitorStatsRepository.updateByMonitorId(monitor.id, { + status: networkResponse.status === true, + responseTime: networkResponse.responseTime ?? 0, + now: Date.now(), + }); return true; } catch (error: unknown) { this.logger.error({ @@ -190,7 +103,14 @@ export class StatusService implements IStatusService { const monitor = await this.monitorsRepository.findById(monitorId, teamId); // Update running stats - this.updateRunningStats(monitor, statusResponse); + const statsOk = await this.updateRunningStats(monitor, statusResponse); + if (!statsOk) { + this.logger.warn({ + service: SERVICE_NAME, + method: "updateMonitorStatus", + message: `Stats update failed for monitor ${monitor.id}`, + }); + } monitor.statusWindow = monitor.statusWindow || []; monitor.statusWindow.push(status); diff --git a/server/src/types/monitorStats.ts b/server/src/types/monitorStats.ts index 97c3cb28c..a3ac914cf 100644 --- a/server/src/types/monitorStats.ts +++ b/server/src/types/monitorStats.ts @@ -1,3 +1,9 @@ +export interface CheckResultInput { + status: boolean; + responseTime: number; + now: number; +} + export interface MonitorStats { id: string; monitorId: string; diff --git a/server/test/unit/services/statusService.test.ts b/server/test/unit/services/statusService.test.ts index f1cfa53d5..66da6dcf4 100644 --- a/server/test/unit/services/statusService.test.ts +++ b/server/test/unit/services/statusService.test.ts @@ -126,144 +126,67 @@ describe("StatusService", () => { // ── updateRunningStats ─────────────────────────────────────────────────── describe("updateRunningStats", () => { - it("creates new stats when none exist", async () => { + // The service now delegates the entire running-stats computation to the repository's + // atomic `updateByMonitorId`, so the service-level tests only verify forwarding and error + // handling. The math (running average, uptimePercentage, timeOfLastFailure state machine) + // is a repository-level concern and is exercised by the repository tests. + + it("forwards a successful check to updateByMonitorId", async () => { const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockRejectedValue(new Error("not found")); - (monitorStatsRepository.create as jest.Mock).mockResolvedValue({}); + (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); const result = await service.updateRunningStats(makeMonitor(), makeStatusResponse({ responseTime: 50 })); expect(result).toBe(true); - expect(monitorStatsRepository.create).toHaveBeenCalledWith( + expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith( + "mon-1", expect.objectContaining({ - monitorId: "mon-1", - totalChecks: 1, - totalUpChecks: 1, - avgResponseTime: 50, - lastResponseTime: 50, + status: true, + responseTime: 50, + now: expect.any(Number), }) ); }); - it("updates existing stats for a successful check", async () => { + it("forwards a failed check to updateByMonitorId", async () => { const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats()); (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); - const result = await service.updateRunningStats(makeMonitor(), makeStatusResponse({ responseTime: 110 })); + const result = await service.updateRunningStats(makeMonitor(), makeStatusResponse({ status: false, responseTime: 100 })); expect(result).toBe(true); - expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith( - "mon-1", - expect.objectContaining({ - totalChecks: 11, - totalUpChecks: 10, - totalDownChecks: 1, - }) - ); + expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ status: false, responseTime: 100 })); }); - it("increments totalDownChecks and resets timeOfLastFailure on failure", async () => { + it("passes 0 when responseTime is undefined", async () => { const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats()); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); - - await service.updateRunningStats(makeMonitor(), makeStatusResponse({ status: false, responseTime: 100 })); - - expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith( - "mon-1", - expect.objectContaining({ - totalDownChecks: 2, - timeOfLastFailure: 0, - }) - ); - }); - - it("sets timeOfLastFailure when status is up and it was previously 0", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats({ timeOfLastFailure: 0 })); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); - - await service.updateRunningStats(makeMonitor(), makeStatusResponse({ status: true })); - - expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith( - "mon-1", - expect.objectContaining({ - timeOfLastFailure: expect.any(Number), - }) - ); - const call = (monitorStatsRepository.updateByMonitorId as jest.Mock).mock.calls[0] as [string, Record]; - expect(call[1].timeOfLastFailure).toBeGreaterThan(0); - }); - - it("updates maxResponseTime when new response is higher", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats({ maxResponseTime: 200 })); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); - - await service.updateRunningStats(makeMonitor(), makeStatusResponse({ responseTime: 300 })); - - expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ maxResponseTime: 300 })); - }); - - it("does not update maxResponseTime when new response is lower", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats({ maxResponseTime: 200 })); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); - - await service.updateRunningStats(makeMonitor(), makeStatusResponse({ responseTime: 50 })); - - expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ maxResponseTime: 200 })); - }); - - it("initializes avgResponseTime when previous avg was 0", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats({ avgResponseTime: 0 })); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); - - await service.updateRunningStats(makeMonitor(), makeStatusResponse({ responseTime: 75 })); - - expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ avgResponseTime: 75 })); - }); - - it("computes running average when avgResponseTime is nonzero", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats({ avgResponseTime: 100, totalChecks: 10 })); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); - - await service.updateRunningStats(makeMonitor(), makeStatusResponse({ responseTime: 210 })); - - const call = (monitorStatsRepository.updateByMonitorId as jest.Mock).mock.calls[0] as [string, Record]; - // (100 * 10 + 210) / 11 = 110 - expect(call[1].avgResponseTime).toBe(110); - }); - - it("leaves avgResponseTime unchanged when responseTime is undefined", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats({ avgResponseTime: 100 })); (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); await service.updateRunningStats(makeMonitor(), makeStatusResponse({ responseTime: undefined })); - expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ avgResponseTime: 100 })); + expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ responseTime: 0 })); }); - it("handles responseTime of 0 (falsy but defined)", async () => { + it("forwards responseTime of 0 as 0 (falsy but defined)", async () => { const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats({ avgResponseTime: 100, maxResponseTime: 200 })); (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); await service.updateRunningStats(makeMonitor(), makeStatusResponse({ responseTime: 0 })); - const call = (monitorStatsRepository.updateByMonitorId as jest.Mock).mock.calls[0] as [string, Record]; - expect(call[1].lastResponseTime).toBe(0); - // responseTime 0 is falsy, so maxResponseTime stays unchanged - expect(call[1].maxResponseTime).toBe(200); + expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ responseTime: 0 })); }); - it("returns false and logs error when an exception is thrown", async () => { + it("coerces a non-true status to false (e.g. undefined)", async () => { + const { service, monitorStatsRepository } = createService(); + (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); + + await service.updateRunningStats(makeMonitor(), makeStatusResponse({ status: undefined as unknown as boolean })); + + expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ status: false })); + }); + + it("returns false and logs error when updateByMonitorId throws", async () => { const { service, logger, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats()); (monitorStatsRepository.updateByMonitorId as jest.Mock).mockRejectedValue(new Error("db write failed")); const result = await service.updateRunningStats(makeMonitor(), makeStatusResponse()); @@ -280,7 +203,6 @@ describe("StatusService", () => { it("logs error with 'Unknown error' for non-Error exceptions", async () => { const { service, logger, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(makeExistingStats()); (monitorStatsRepository.updateByMonitorId as jest.Mock).mockRejectedValue("string error"); const result = await service.updateRunningStats(makeMonitor(), makeStatusResponse()); @@ -288,17 +210,6 @@ describe("StatusService", () => { expect(result).toBe(false); expect(logger.error).toHaveBeenCalledWith(expect.objectContaining({ message: "Unknown error", stack: undefined })); }); - - it("creates new stats when findByMonitorId resolves to null", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.findByMonitorId as jest.Mock).mockResolvedValue(null); - (monitorStatsRepository.create as jest.Mock).mockResolvedValue({}); - - const result = await service.updateRunningStats(makeMonitor(), makeStatusResponse({ responseTime: 50, status: true })); - - expect(result).toBe(true); - expect(monitorStatsRepository.create).toHaveBeenCalled(); - }); }); // ── updateMonitorStatus ────────────────────────────────────────────────── From e1b00ce73509b3f7ed44a0b8aa29586a51b17dd3 Mon Sep 17 00:00:00 2001 From: Alex Holliday Date: Mon, 13 Apr 2026 11:58:24 -0700 Subject: [PATCH 2/2] dedupe monitor stats, run migrations before index creation --- server/src/db/MongoDB.ts | 8 +- ...anupDuplicateMonitorStatsForUniqueIndex.ts | 78 +++++++++++++++++++ server/src/db/migration/index.ts | 2 + 3 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 server/src/db/migration/0006_cleanupDuplicateMonitorStatsForUniqueIndex.ts diff --git a/server/src/db/MongoDB.ts b/server/src/db/MongoDB.ts index 8aea56279..59d2a32b7 100755 --- a/server/src/db/MongoDB.ts +++ b/server/src/db/MongoDB.ts @@ -35,6 +35,12 @@ class MongoDB implements IDb { setDefaultsOnInsert: true, } ); + // Run migrations BEFORE syncIndexes. The 0006 dedupe migration must run before + // the unique index on MonitorStats.monitorId is enforced, otherwise databases + // with stray duplicates (caused by the pre-fix race in updateRunningStats) + // will crash at startup when the unique index fails to build. + await runMigrations(); + // Sync indexes const models = mongoose.modelNames(); for (const modelName of models) { @@ -47,8 +53,6 @@ class MongoDB implements IDb { service: SERVICE_NAME, method: "connect", }); - - await runMigrations(); } catch (error: unknown) { this.logger.error({ message: error instanceof Error ? error.message : "Unknown error", diff --git a/server/src/db/migration/0006_cleanupDuplicateMonitorStatsForUniqueIndex.ts b/server/src/db/migration/0006_cleanupDuplicateMonitorStatsForUniqueIndex.ts new file mode 100644 index 000000000..814b19776 --- /dev/null +++ b/server/src/db/migration/0006_cleanupDuplicateMonitorStatsForUniqueIndex.ts @@ -0,0 +1,78 @@ +import { MonitorStatsModel } from "../models/MonitorStats.js"; +import { logger } from "@/utils/logger.js"; + +/** + * Second-pass dedupe of MonitorStats before the unique index on monitorId is enforced. + * + * 0003 ran the same dedupe, but the pre-fix race in StatusService.updateRunningStats + * could create new duplicates after 0003 completed. This migration re-runs the dedupe so + * the unique index added in the same PR can be built without failing. + * + * Keeps the document with the highest totalChecks for each monitorId (most data) and + * deletes the rest. Falls back to most recent updatedAt when totalChecks tie. + */ +export async function cleanupDuplicateMonitorStatsForUniqueIndex(): Promise { + const SERVICE_NAME = "Migration:CleanupDuplicateMonitorStatsForUniqueIndex"; + + try { + logger.info({ service: SERVICE_NAME, message: "Starting pre-unique-index dedupe of MonitorStats" }); + + const duplicates = await MonitorStatsModel.aggregate([ + { + $group: { + _id: "$monitorId", + docs: { $push: { id: "$_id", totalChecks: "$totalChecks", updatedAt: "$updatedAt" } }, + count: { $sum: 1 }, + }, + }, + { $match: { count: { $gt: 1 } } }, + ]); + + if (duplicates.length === 0) { + logger.info({ service: SERVICE_NAME, message: "No duplicate MonitorStats found" }); + return; + } + + logger.info({ + service: SERVICE_NAME, + message: `Found ${duplicates.length} monitors with duplicate stats`, + }); + + let totalDeleted = 0; + + type DocInfo = { id: string; totalChecks: number; updatedAt: Date }; + + for (const duplicate of duplicates) { + const monitorId = duplicate._id; + const docs: DocInfo[] = duplicate.docs; + + // Sort by totalChecks desc, then updatedAt desc — keep the winner, delete the rest. + docs.sort((a, b) => { + const checksDiff = (b.totalChecks ?? 0) - (a.totalChecks ?? 0); + if (checksDiff !== 0) return checksDiff; + return new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime(); + }); + + const toDelete = docs.slice(1).map((doc) => doc.id); + + if (toDelete.length > 0) { + const result = await MonitorStatsModel.deleteMany({ _id: { $in: toDelete } }); + totalDeleted += result.deletedCount ?? 0; + + logger.debug({ + service: SERVICE_NAME, + message: `Deleted ${result.deletedCount} duplicate stats for monitor ${monitorId}`, + }); + } + } + + logger.info({ + service: SERVICE_NAME, + message: `Cleanup complete. Deleted ${totalDeleted} duplicate MonitorStats documents`, + }); + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + logger.error({ service: SERVICE_NAME, message: `Error during MonitorStats dedupe: ${errorMessage}` }); + throw error; + } +} diff --git a/server/src/db/migration/index.ts b/server/src/db/migration/index.ts index f1a672af9..5b74a96fc 100644 --- a/server/src/db/migration/index.ts +++ b/server/src/db/migration/index.ts @@ -4,6 +4,7 @@ import { cleanupDuplicateMonitorStats } from "./0003_cleanupDuplicateMonitorStat import { fixInfrastructureThresholds } from "./0004_fixInfrastructureThresholds.js"; import MigrationModel from "../models/Migration.js"; import { migrateStatusPageTypeToArray } from "./0005_migrateStatusPageTypeToArray.js"; +import { cleanupDuplicateMonitorStatsForUniqueIndex } from "./0006_cleanupDuplicateMonitorStatsForUniqueIndex.js"; import type { ILogger } from "@/utils/logger.js"; type MigrationEntry = { @@ -17,6 +18,7 @@ const migrations: MigrationEntry[] = [ { name: "0003_cleanupDuplicateMonitorStats", execute: cleanupDuplicateMonitorStats }, { name: "0004_fixInfrastructureThresholds", execute: fixInfrastructureThresholds }, { name: "0005_migrateStatusPageTypeToArray", execute: migrateStatusPageTypeToArray }, + { name: "0006_cleanupDuplicateMonitorStatsForUniqueIndex", execute: cleanupDuplicateMonitorStatsForUniqueIndex }, ]; const runMigrations = async (logger?: ILogger) => {