Repository navigation
Heartbeat retention 60 days - #1079
Conversation
Add a single-field TTL index on heartbeat timestamps while preserving the existing query index. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f27d8f52-9620-4ee2-a564-ec4870a5f840
Clear the queued-check marker when heartbeat history is missing. A new heartbeat restarts inactive monitoring without duplicating active checks. Preserve online/offline notifications and cover recovery with regression tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f27d8f52-9620-4ee2-a564-ec4870a5f840
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 13 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
[Medium impact] The changes affect heartbeat history and checks within the MongoDB-backed monitoring workflow. Fix the duplicate-check and lost-check races before merging. Findings
SummaryAdds a 60-day MongoDB TTL index for heartbeat history.
The restart and stop paths need coordination to prevent duplicate or missing checks. A queue error also skips updating the phone's online state. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Heartbeat arrives] --> B[Save heartbeat]
B --> C{Check inactive?}
C -->|Yes| D[Schedule check]
C -->|No| E[Update phone online]
D -->|Success| E
D -->|Error| F[Return before online update]
G[Check runs] --> H{History exists?}
H -->|Yes| I[Check phone and schedule next check]
H -->|No| J[Clear QueueID and stop]
Reviews (1) · Last reviewed commit: "fix(api): restart stopped checks when he..." · Reviewed by Greptile |
| if monitor.QueueID == "" || monitor.RequiresCheck() { | ||
| if err = service.scheduleHeartbeatCheck(ctx, heartbeat.Timestamp, &HeartbeatMonitorParams{ |
There was a problem hiding this comment.
Two overlapping heartbeats can both load an inactive monitor before either saves its new QueueID. Both then schedule a check. Each check calls Monitor, which schedules another check without checking whether it still owns QueueID. This creates lasting duplicate checks and missed-heartbeat notifications.
Claim the restart atomically, and add a test with overlapping Store calls.
| if stacktrace.GetCode(err) == repositories.ErrCodeNotFound { | ||
| ctxLogger.Info(fmt.Sprintf("heartbeat history does not exist for userID [%s] and owner [%s], stopping check", params.UserID, params.Owner)) | ||
| if err = service.monitorRepository.UpdateQueueID(ctx, monitor.ID, ""); err != nil { | ||
| return service.tracer.WrapErrorSpan(span, stacktrace.Propagatef(err, "cannot clear heartbeat check queue for monitor [%s]", monitor.ID)) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
Returning phones can lose checks
After Last returns not found, Store can save a heartbeat and read the still-populated, recently updated QueueID, so it does not restart checks. This branch then clears QueueID and exits. If the phone goes offline again before sending another heartbeat, no task remains to detect it.
Coordinate stopping and restarting checks so a heartbeat arriving in this window cannot be missed.
No description provided.