Skip to content

Commit

Permalink
fix: updating SafeToRetry() function to retry on wrapped errors
Browse files Browse the repository at this point in the history
There is a rather unique bug when a single connection is being slammed
with queries. When statement caching is enabled, and the
`deallocateInvalidatedCachedStatements()` method is called to clean up,
the `Pipeline.Sync()` method has the ability to error out due to the
connection being in-use.

`connLockError` already implements the inexplicit `SafeToRetry() bool()`
interface, which is then checked by the `SafeToRetry()` function. In the
event the connection is locked due to being in use, the
`deallocateInvalidatedCachedStatements()` method can return a wrapped
error that looks like:
```
failed to deallocate cached statement(s): conn busy
```

When the `stdlib` PGX wrapper is used, it is possible for this to not
be retried, as it is also taking advantage of this `SafeToRetry()`
function. In order to correct this and prevent future similar errors,
the `SafeToRetry()` function has been updated to check against the
wrapped errors as well.
  • Loading branch information
tjasko authored and jackc committed May 9, 2024
1 parent 7fb9e14 commit 4e2e7a0
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import (

// SafeToRetry checks if the err is guaranteed to have occurred before sending any data to the server.
func SafeToRetry(err error) bool {
if e, ok := err.(interface{ SafeToRetry() bool }); ok {
return e.SafeToRetry()
var retryableErr interface{ SafeToRetry() bool }
if errors.As(err, &retryableErr) {
return retryableErr.SafeToRetry()
}
return false
}
Expand Down

0 comments on commit 4e2e7a0

Please sign in to comment.