Skip to content

Fix reconnect goroutine leaks + only log errors after 30s of failed reconnects#27

Open
dselans wants to merge 1 commit into
mainfrom
dselans/reconnect-leak-and-grace
Open

Fix reconnect goroutine leaks + only log errors after 30s of failed reconnects#27
dselans wants to merge 1 commit into
mainfrom
dselans/reconnect-leak-and-grace

Conversation

@dselans

@dselans dselans commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Weekly AMQ maintenance reboots currently cause every consumer to spam errChan (which services log as errors) and leak goroutines. This PR fixes the reconnect path:

  • Fixes goroutine leak in Publish() (unbuffered chanErr/chanDone + missing return meant every failed publish leaked a goroutine blocked on chanDone)
  • Fixes goroutine leak in writeError() (spawned goroutines that block forever when errChan is full/unread); now a non-blocking send
  • reconnect() closes the old connection if still alive, so reconnects over a live conn (force-reconnect, channel-level errors, back-to-back reconnect signals) no longer leak amqp connections + their goroutines
  • New Options.ReconnectErrorAfterSec (default 30s): reconnect failures log at warn and only escalate to error after failing for 30s
  • Channel setup failure after reconnect is retried instead of panicking
  • Nil-acknowledger disconnects are no longer written to errChan (they are not consume errors; the watcher handles/logs reconnects)
  • DefaultRetryReconnectSec lowered 60s -> 5s so services recover quickly after a reboot
  • Watcher exits on Stop()/Close() instead of reconnecting a shutdown client

Tests: updated nil-ack spec for new behavior, added regression specs for both leaks, old-conn cleanup, and warn-then-error escalation (verified the publish leak spec fails against the old code).

- Publish() no longer leaks a goroutine per failed publish (unbuffered
  chanErr/chanDone + missing return)
- writeError() no longer spawns a goroutine that blocks forever when
  errChan is unbuffered/full
- reconnect() closes the old connection (if alive) so repeated
  reconnects do not leak amqp connections/goroutines
- Reconnect failures log at warn level and only escalate to error after
  ReconnectErrorAfterSec (default 30s) - avoids alerting on routine
  server maintenance restarts
- Channel setup failure after reconnect retries instead of panicking
- Disconnects (nil acknowledger) are no longer written to errChan
- DefaultRetryReconnectSec lowered 60s -> 5s
- Watcher exits on shutdown instead of reconnecting a closed client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants