Skip to content

Commit

Permalink
compact: unwrap the original error as well (#2637)
Browse files Browse the repository at this point in the history
* compact: unwrap the original error as well

Just like in IsHaltError(), let's unwrap the given error to see if is
`terrors.MultiError`. This lets us detect the retryable errors as well:

* `BucketCompactor.Compact` returns `terrors.MultiError` of wrapped
errors;
* `compactMainFn` wraps the previously returned error again.

So, we need to unwrap it at the beginning, iterate over it, and then
unwrap again which is what the code does with this change.

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

* compact: test: add case for wrapped retryable error

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

* CHANGELOG: update

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>
  • Loading branch information
GiedriusS authored May 21, 2020
1 parent 48c80d1 commit 134ef5d
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ We use *breaking* word for marking changes that are not backward compatible (rel

## Unreleased

### Fixed

* [#2637](https://github.com/thanos-io/thanos/pull/2637) Compact: detect retryable errors that are inside of a wrapped `tsdb.MultiError`

## [v0.13.0](https://github.com/thanos-io/thanos/releases) - IN PROGRESS

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion pkg/compact/compact.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ func (e RetryError) Error() string {
// IsRetryError returns true if the base error is a RetryError.
// If a multierror is passed, all errors must be retriable.
func IsRetryError(err error) bool {
if multiErr, ok := err.(terrors.MultiError); ok {
if multiErr, ok := errors.Cause(err).(terrors.MultiError); ok {
for _, err := range multiErr {
if _, ok := errors.Cause(err).(RetryError); !ok {
return false
Expand Down
2 changes: 2 additions & 0 deletions pkg/compact/compact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ func TestRetryMultiError(t *testing.T) {
errs = terrors.MultiError{retryErr}
testutil.Assert(t, IsRetryError(errs), "if all errors are retriable this should return true")

testutil.Assert(t, IsRetryError(errors.Wrap(errs, "wrap")), "retry error with wrap")

errs = terrors.MultiError{nonRetryErr, retryErr}
testutil.Assert(t, !IsRetryError(errs), "mixed errors should return false")
}
Expand Down

0 comments on commit 134ef5d

Please sign in to comment.