From 9d4d19cb222c7f0a66dd3beebca918232b55a4ab Mon Sep 17 00:00:00 2001 From: Aloento <11802769+Aloento@users.noreply.github.com> Date: Sun, 4 Oct 2026 21:06:37 +0200 Subject: [PATCH] Refetch a maintenance only when its status will change The scan now decides from the batch-loaded statuses whether the computed status differs and refetches only then, instead of one GetIncident per event. Every event is still scanned, so out-of-band DB edits stay reconciled. The checker's event query no longer preloads components, which it never reads. --- internal/checker/maintenance.go | 72 ++++++++++++++++------------ internal/checker/maintenance_test.go | 55 +++++++++++++++++++++ internal/db/event_types.go | 5 -- tests/checker_notifications_test.go | 24 ++++++++++ 4 files changed, 121 insertions(+), 35 deletions(-) diff --git a/internal/checker/maintenance.go b/internal/checker/maintenance.go index ebbd23a..61f0630 100644 --- a/internal/checker/maintenance.go +++ b/internal/checker/maintenance.go @@ -80,43 +80,55 @@ func (ch *Checker) CheckMaintenance() error { return nil } +// needsRefetch reports whether the batch-loaded state already agrees with the +// status the checker would compute. When it does, the event is steady-state and +// the per-event refetch can be skipped; when it does not, a fresh read is needed +// before the read-modify-write. +func needsRefetch(mn *db.Incident) bool { + return mn.Status != calculateCurrentMntStatus(calculateMntStatusHistory(mn), mn) +} + func (ch *Checker) processMaintenance(mn *db.Incident) error { - // Refetch immediately before the read-modify-write. The bulk - // GetMaintenances above is N items old by the time we reach item N; - // using its preloaded state for the version check races concurrent - // API edits. A single fresh read shrinks the race window from - // "duration of the whole tick" to "one DB round-trip", which makes - // ErrVersionConflict effectively unreachable without a retry loop. - mn, err := ch.db.GetIncident(int(mn.ID)) + // Decide from the batch-loaded state whether the status will change. Only + // then refetch: a fresh read immediately before the read-modify-write + // shrinks the version-conflict window, and the write is the only place the + // backfilled statuses persist. Steady-state events (no change) skip the + // refetch, but every event is still scanned so manual DB edits are caught. + if !needsRefetch(mn) { + return nil + } + + fresh, err := ch.db.GetIncident(int(mn.ID)) if err != nil { return fmt.Errorf("refetch maintenance %d: %w", mn.ID, err) } - actualStatus := ch.evaluateAndFixMntStatus(mn) - - if mn.Status != actualStatus { - oldStatus := mn.Status - mn.Status = actualStatus - // The modify + enqueue share one transaction: on a version conflict the - // whole thing rolls back and no notification is published. - txErr := ch.db.WithTx(context.Background(), func(tx *db.Tx) error { - if modErr := ch.db.ModifyIncidentTx(tx, mn); modErr != nil { - return modErr - } - return ch.notifier.PublishTx(context.Background(), tx, notification.Change{ - IncidentID: mn.ID, - Title: strDeref(mn.Text), - OldStatus: oldStatus, - NewStatus: mn.Status, - ContactEmail: strDeref(mn.ContactEmail), - Actor: notification.ActorChecker, - }) - }) - if txErr != nil { - return fmt.Errorf("update maintenance %d: %w", mn.ID, txErr) + actualStatus := ch.evaluateAndFixMntStatus(fresh) + if fresh.Status == actualStatus { + return nil + } + + oldStatus := fresh.Status + fresh.Status = actualStatus + // The modify + enqueue share one transaction: on a version conflict the + // whole thing rolls back and no notification is published. + txErr := ch.db.WithTx(context.Background(), func(tx *db.Tx) error { + if modErr := ch.db.ModifyIncidentTx(tx, fresh); modErr != nil { + return modErr } - ch.notifier.Notify() // wake the worker after the commit + return ch.notifier.PublishTx(context.Background(), tx, notification.Change{ + IncidentID: fresh.ID, + Title: strDeref(fresh.Text), + OldStatus: oldStatus, + NewStatus: fresh.Status, + ContactEmail: strDeref(fresh.ContactEmail), + Actor: notification.ActorChecker, + }) + }) + if txErr != nil { + return fmt.Errorf("update maintenance %d: %w", mn.ID, txErr) } + ch.notifier.Notify() // wake the worker after the commit return nil } diff --git a/internal/checker/maintenance_test.go b/internal/checker/maintenance_test.go index 8f1b459..3af6096 100644 --- a/internal/checker/maintenance_test.go +++ b/internal/checker/maintenance_test.go @@ -11,6 +11,61 @@ import ( "github.com/stackmon/otc-status-dashboard/internal/event" ) +func TestNeedsRefetch(t *testing.T) { + future := time.Now().UTC().Add(24 * time.Hour) + past := time.Now().UTC().Add(-24 * time.Hour) + + tests := []struct { + name string + mn *db.Incident + expected bool + }{ + { + name: "planned with future start is steady-state", + mn: &db.Incident{ + Status: event.MaintenancePlanned, + StartDate: &future, + Statuses: []db.IncidentStatus{{Status: event.MaintenancePlanned}}, + }, + expected: false, + }, + { + name: "planned with past start needs refetch", + mn: &db.Incident{ + Status: event.MaintenancePlanned, + StartDate: &past, + Statuses: []db.IncidentStatus{{Status: event.MaintenancePlanned}}, + }, + expected: true, + }, + { + name: "reviewed auto-approves to planned, needs refetch", + mn: &db.Incident{ + Status: event.MaintenanceReviewed, + StartDate: &future, + Statuses: []db.IncidentStatus{{Status: event.MaintenanceReviewed}}, + }, + expected: true, + }, + { + name: "completed with past end is steady-state", + mn: &db.Incident{ + Status: event.MaintenanceCompleted, + StartDate: &past, + EndDate: &past, + Statuses: []db.IncidentStatus{{Status: event.MaintenanceCompleted}}, + }, + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, needsRefetch(tc.mn)) + }) + } +} + func TestCalculateCurrentMntStatus(t *testing.T) { future := time.Now().UTC().Add(24 * time.Hour) farFuture := future.Add(48 * time.Hour) diff --git a/internal/db/event_types.go b/internal/db/event_types.go index 1aaefba..d574873 100644 --- a/internal/db/event_types.go +++ b/internal/db/event_types.go @@ -5,8 +5,6 @@ import ( entsql "entgo.io/ent/dialect/sql" - "github.com/stackmon/otc-status-dashboard/ent" - "github.com/stackmon/otc-status-dashboard/ent/component" "github.com/stackmon/otc-status-dashboard/ent/incident" ) @@ -16,9 +14,6 @@ func (db *DB) getEventsByType(eventType incident.Type, order entsql.OrderTermOpt query := db.e.Incident.Query(). Where(incident.TypeEQ(eventType)). - WithComponents(func(q *ent.ComponentQuery) { - q.Select(component.FieldID) - }). Order(incident.ByID(order)) rows, err := query.All(ctx) diff --git a/tests/checker_notifications_test.go b/tests/checker_notifications_test.go index fda31ec..3026a5d 100644 --- a/tests/checker_notifications_test.go +++ b/tests/checker_notifications_test.go @@ -83,3 +83,27 @@ func TestChecker_NoTransition_EnqueuesNothing(t *testing.T) { assert.Equal(t, int64(0), outboxCount(t, g, eventID), "no notification without a real transition") } + +// TestChecker_SteadyState_SkipsRefetch verifies that a steady-state maintenance +// (status already matches the computed target) is left untouched: no version +// bump and no outbox rows, even across repeated scans. +func TestChecker_SteadyState_SkipsRefetch(t *testing.T) { + truncateIncidents(t) + + r := initTests(t) + resp := createEventOK(t, r, maintenanceData(), adminToken) // admin -> planned (future start) + eventID := resp.Result[0].IncidentID + + g := openRawDB(t) + inc := getEventOK(t, r, eventID, adminToken) + initialVersion := eventVersion(inc) + + chk := newTestChecker(t) + + require.NoError(t, chk.CheckMaintenance()) + require.NoError(t, chk.CheckMaintenance()) + + after := getEventOK(t, r, eventID, adminToken) + assert.Equal(t, initialVersion, eventVersion(after), "steady-state scan must not bump the version") + assert.Equal(t, int64(0), outboxCount(t, g, eventID), "steady-state scan must not enqueue") +}