DB Retry Behavior #144
DB Retry Behavior #144
Conversation
internal/storage/conn.go
Outdated
| break | ||
| } | ||
| secondsToWait := i + 1 | ||
| c.logger.Log().Warning("Database Connection Err: ", err) |
There was a problem hiding this comment.
A bit of a nitpick, but those two log entries are not guaranteed to reach the physical log stream together. Why not put it all in a single message?
There was a problem hiding this comment.
No particular reason, besides I thought you would want them separated out because one is an error and one is an info statement. You never specified what you wanted, so I had to assume brother. so I went with a warning message because the error was causing a failure. I don't really have a strong opinion. So be specific and I will change it to whatever you want it to be.
There was a problem hiding this comment.
A single Warning containing both a database error details and a retry details is fine here
|
@sayplastic How does coverage decrease if the function adjusted has a unit test covering it? |
Test coverage is not binary, you added a logical branch to the function that the existing test does not cover. |
|
@tiger5226, @sayplastic says you can and should merge this |
|
All clear. I was just curious. I did not know if was that detailed of a check. Good to know. |
… Retry periodically, with increasing intervals before finally returning an error to connect to the database.
Don't just error on db connections. DBs lose connections, it happens. Retry periodically, with increasing intervals before finally returning an error to connect to the database.
This is also beneficial for our container compositions. If postgres does not startup in time, it will retry.