fix(status-history): order same-second events by id

Status rows written in the same second have equal timestamps, and the
batched query returns them in an order that depends on which other
resources are in the request. When a recovery event sorted before the
failure it followed, the resource stayed unhealthy until its next event.

Break timestamp ties by id in the queries and in computeBuckets.

Fixes #3994

server/lib/statusHistory.test.ts

@@ -0,0 +1,58 @@
+import { computeBuckets } from "./statusHistory";
+import { assertEquals } from "@test/assert";
+
+function event(id: number, timestamp: number, status: string) {
+    return {
+        entityType: "resource",
+        entityId: 1,
+        orgId: "org",
+        status,
+        timestamp,
+        id
+    };
+}
+
+// Regression test for https://github.com/fosrl/pangolin/issues/3994
+function testSameSecondEventsUseInsertionOrder() {
+    console.log("Running computeBuckets same-second ordering tests...");
+
+    // Two days ago the resource went unhealthy and recovered within the same
+    // second. The database may return the two rows in either order.
+    const nowSec = Math.floor(Date.now() / 1000);
+    const flap = nowSec - 2 * 86400;
+    const unhealthy = event(1, flap, "unhealthy");
+    const healthy = event(2, flap, "healthy");
+
+    for (const events of [
+        [unhealthy, healthy],
+        [healthy, unhealthy]
+    ]) {
+        const { buckets, totalDowntime } = computeBuckets(
+            events,
+            5,
+            "healthy",
+            0
+        );
+        const order = events.map((e) => e.id).join(",");
+        assertEquals(
+            totalDowntime,
+            0,
+            `No downtime when the later event is healthy (rows ${order})`
+        );
+        assertEquals(
+            buckets.at(-1)!.status,
+            "good",
+            `Today should be good (rows ${order})`
+        );
+    }
+
+    console.log("All computeBuckets same-second ordering tests passed!");
+}
+
+try {
+    testSameSecondEventsUseInsertionOrder();
+    console.log("All tests passed successfully!");
+} catch (error) {
+    console.error("Test failed:", error);
+    process.exit(1);
+}

server/lib/statusHistory.ts

@@ -56,7 +56,7 @@ export async function getCachedStatusHistory(
                 gte(statusHistory.timestamp, startSec)
             )
         )
-        .orderBy(asc(statusHistory.timestamp));
+        .orderBy(asc(statusHistory.timestamp), asc(statusHistory.id));
 
     // Fetch the last known state before the window so that entities that
     // haven't changed status recently still show the correct status rather
@@ -71,7 +71,7 @@ export async function getCachedStatusHistory(
                 lt(statusHistory.timestamp, startSec)
             )
         )
-        .orderBy(desc(statusHistory.timestamp))
+        .orderBy(desc(statusHistory.timestamp), desc(statusHistory.id))
         .limit(1);
 
     const priorStatus = lastKnownEvent?.status ?? null;
@@ -168,6 +168,14 @@ export function computeBuckets(
     // don't drift as the cache expires and is recomputed
     const todayMidnightSec = localMidnightSec(tzOffsetMinutes);
 
+    // Several status changes can land in the same second (e.g. a resource
+    // going degraded -> healthy as each target reports in), and the database
+    // does not keep a stable order for rows with equal timestamps. Order by id
+    // as well so the last event of the second is the one that was written last.
+    events = [...events].sort(
+        (a, b) => a.timestamp - b.timestamp || a.id - b.id
+    );
+
     const buckets: StatusHistoryDayBucket[] = [];
     let totalDowntime = 0;
 
@@ -328,7 +336,7 @@ export async function getBatchedStatusHistory(
                 gte(statusHistory.timestamp, startSec)
             )
         )
-        .orderBy(asc(statusHistory.timestamp));
+        .orderBy(asc(statusHistory.timestamp), asc(statusHistory.id));
 
     // Fetch the last known state before the window so that entities that
     // haven't changed status recently still show the correct status rather
@@ -349,7 +357,7 @@ export async function getBatchedStatusHistory(
             status: statusHistory.status,
             timestamp: statusHistory.timestamp,
             row_number:
-                sql<number>`ROW_NUMBER() OVER (PARTITION BY ${statusHistory.entityId} ORDER BY ${statusHistory.timestamp} DESC)`.as(
+                sql<number>`ROW_NUMBER() OVER (PARTITION BY ${statusHistory.entityId} ORDER BY ${statusHistory.timestamp} DESC, ${statusHistory.id} DESC)`.as(
                     "row_number"
                 )
         })