$ go test -race -run "TestRunStopRace" github.com/coder/coder/v2/coderd/notifications -count=10000 -parallel $(nproc)--- FAIL: TestRunStopRace (0.00s)    t.go:106: 2025-06-06 02:44:39.348 [debu]  notifications-manager: notification manager started    t.go:106: 2025-06-06 02:44:39.348 [debu]  notifications-manager: graceful stop requested    t.go:106: 2025-06-06 02:44:39.348 [debu]  notifications-manager: notification manager stopped    t.go:115: 2025-06-06 02:44:39.348 [erro]  notifications-manager: notification manager stopped with error ...        error= manager already closed:                   github.com/coder/coder/v2/coderd/notifications.(*Manager).loop                       /home/coder/coder/coderd/notifications/manager.go:166         *** slogtest: log detected at level ERROR; TEST FAILURE ***--- FAIL: TestRunStopRace (0.00s)    t.go:106: 2025-06-06 02:44:41.632 [debu]  notifications-manager: notification manager started    t.go:106: 2025-06-06 02:44:41.632 [debu]  notifications-manager: graceful stop requested    t.go:106: 2025-06-06 02:44:41.632 [debu]  notifications-manager: notification manager stopped    t.go:115: 2025-06-06 02:44:41.633 [erro]  notifications-manager: notification manager stopped with error ...        error= manager already closed:                   github.com/coder/coder/v2/coderd/notifications.(*Manager).loop                       /home/coder/coder/coderd/notifications/manager.go:166         *** slogtest: log detected at level ERROR; TEST FAILURE ***FAILFAIL    github.com/coder/coder/v2/coderd/notifications  6.847sFAIL

These error logs are caused as a result of theManagerRun start operation being asynchronous. In the flaking test case we immediately callStop afterRun. It's possible forStop to be scheduled to completion before the goroutine spawned byRun callsloop and checksclosed. If this happens,loop returns an error and produces the error log.

We'll address this by replacing this specific error log with a warning log.

$ go test -run "TestRunStopRace" github.com/coder/coder/v2/coderd/notifications -count=10000 -parallel $(nproc)ok      github.com/coder/coder/v2/coderd/notifications  1.294s$ go test -race github.com/coder/coder/v2/coderd/notifications -count=100 -parallel $(nproc)ok      github.com/coder/coder/v2/coderd/notifications  26.525s

ethanndickson force-pushed theethan/remove-closed-check branch frombf720c0 tob7e88f0Compare

June 6, 2025 03:53

This was referencedJun 6, 2025

chore(vpn): send ping results over tunnel#18200

Merged

chore(vpn): send info, debug logs over tunnel#18240

Merged

Copy link

MemberAuthor

ethanndickson commentedJun 6, 2025•
edited
Loading

fix: remove error log when notification manager is already closed #18264 👈(View in Graphite)
main

This stack of pull requests is managed byGraphite. Learn more aboutstacking.

github-actionsbot assignedethanndickson

Jun 6, 2025

ethanndickson changed the base branch fromethan/send-ping-results tomain

June 6, 2025 03:53

ethanndickson commented

Jun 6, 2025

View reviewed changes

coderd/notifications/manager.goShow resolvedHide resolved

ethanndickson requested a review frommafredri

June 6, 2025 04:13

ethanndickson marked this pull request as ready for review

June 6, 2025 04:14

fix: remove closed check when starting notification manager

f8f5e68

ethanndickson force-pushed theethan/remove-closed-check branch fromb7e88f0 tof8f5e68Compare

June 6, 2025 05:41

ethanndickson added2 commits

June 6, 2025 05:42

reorder log

9617776

silence manager already closed error logs

116e9c2

ethanndickson changed the title~~fix: remove closed check when starting notification manager~~fix: remove error log when notification manager is closed on startup

Jun 6, 2025

ethanndickson changed the title~~fix: remove error log when notification manager is closed on startup~~fix: remove error log when notification manager is already closed

Jun 6, 2025