Closed krancour closed 2 years ago
netlify[bot]
commented
2 years ago ✔️ Deploy Preview for brigade-docs ready!
🔨 Explore the source changes: aef65439b2dc0db30a4e0c79ec840714f1688a25
🔍 Inspect the deploy log: https://app.netlify.com/sites/brigade-docs/deploys/620e9520bbf0f80008fdd05d
😎 Browse the preview: https://deploy-preview-1826--brigade-docs.netlify.app
netlify[bot]
commented
2 years ago ✔️ Deploy Preview for brigade-docs ready!
🔨 Explore the source changes: 71e9e92543c556e528911b3954b6ddb0a4ad0aa2
🔍 Inspect the deploy log: https://app.netlify.com/sites/brigade-docs/deploys/620e9801edd3190007351dca
😎 Browse the preview: https://deploy-preview-1826--brigade-docs.netlify.app
I've discovered the source of a known bug while investigating #1821.
This AMQP client connect/re-connect logic is managed by
retries.ManageRetries(...). The retry logic works well when refreshing a stale connection, however, if there is no connection and it attempts to dial the message server for the first time, and fails, the AMQP client is no longer nil, but also is not fully initialized. On the next retry, the call tow.amqpClient.Close()will trigger a nil pointer dereference. 😦We've frequently seen panics related to this in API server startup logs.
The reason this hasn't been a bigger problem than it has been to date is that Artemis usually comes up faster than MongoDB. The API server ends up crashing and restarting itself several times before MongoDB is up and running, and by the time it is, Artemis is usually up and running and the very first attempt to dial the messaging server succeeds without a hitch.
Once #1821 is addressed, DB unavailability won't trigger immediate API server restarts, which means, when combined with the above, that we'll never get a good connection to the message server.
This PR fixes the nil pointer dereference so the retry logic for dialing the message server works as intended.