-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
changefeedccl: switch retryable errors back to a whitelist
For a while, the cdc/crdb-chaos and cdc/sink-chaos roachtests have been failing because an error that should be marked as retryable wasn't. As a result of the discussion in #35974, I tried switching from a whitelist (retryable error) to a blacklist (terminal error) in #36132, but on reflection this doesn't seem like a great idea. We added a safety net to prevent false negatives from retrying indefinitely but it was immediately apparent that this meant we needed to tune the retry loop parameters. Better is to just do the due diligence of investigating the errors that should be retried and retrying them. The commit is intended for backport into 19.1 once it's baked for a bit. Closes #35974 Closes #36018 Closes #36019 Closes #36432 Release note (bug fix): `CHANGEFEED` now retry instead of erroring in more situations
- Loading branch information
Showing
7 changed files
with
201 additions
and
113 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| // Copyright 2019 The Cockroach Authors. | ||
| // | ||
| // Licensed as a CockroachDB Enterprise file under the Cockroach Community | ||
| // License (the "License"); you may not use this file except in compliance with | ||
| // the License. You may obtain a copy of the License at | ||
| // | ||
| // https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt | ||
|
|
||
| package changefeedccl | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strings" | ||
| ) | ||
|
|
||
| const retryableErrorString = "retryable changefeed error" | ||
|
|
||
| type retryableError struct { | ||
| wrapped error | ||
| } | ||
|
|
||
| // MarkRetryableError wraps the given error, marking it as retryable to | ||
| // changefeeds. | ||
| func MarkRetryableError(e error) error { | ||
| return &retryableError{wrapped: e} | ||
| } | ||
|
|
||
| // Error implements the error interface. | ||
| func (e *retryableError) Error() string { | ||
| return fmt.Sprintf("%s: %s", retryableErrorString, e.wrapped.Error()) | ||
| } | ||
|
|
||
| // Cause implements the github.com/pkg/errors.causer interface. | ||
| func (e *retryableError) Cause() error { return e.wrapped } | ||
|
|
||
| // Unwrap implements the github.com/golang/xerrors.Wrapper interface, which is | ||
| // planned to be moved to the stdlib in go 1.13. | ||
| func (e *retryableError) Unwrap() error { return e.wrapped } | ||
|
|
||
| // IsRetryableError returns true if the supplied error, or any of its parent | ||
| // causes, is a IsRetryableError. | ||
| func IsRetryableError(err error) bool { | ||
| for { | ||
| if err == nil { | ||
| return false | ||
| } | ||
| if _, ok := err.(*retryableError); ok { | ||
| return true | ||
| } | ||
| errStr := err.Error() | ||
| if strings.Contains(errStr, retryableErrorString) { | ||
| // If a RetryableError occurs on a remote node, DistSQL serializes it such | ||
| // that we can't recover the structure and we have to rely on this | ||
| // unfortunate string comparison. | ||
| return true | ||
| } | ||
| if strings.Contains(errStr, `rpc error`) { | ||
| // When a crdb node dies, any DistSQL flows with processors scheduled on | ||
| // it get an error with "rpc error" in the message from the call to | ||
| // `(*DistSQLPlanner).Run`. | ||
| return true | ||
| } | ||
| if e, ok := err.(interface{ Unwrap() error }); ok { | ||
| err = e.Unwrap() | ||
| continue | ||
| } | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| // MaybeStripRetryableErrorMarker performs some minimal attempt to clean the | ||
| // RetryableError marker out. This won't do anything if the RetryableError | ||
| // itself has been wrapped, but that's okay, we'll just have an uglier string. | ||
| func MaybeStripRetryableErrorMarker(err error) error { | ||
| if e, ok := err.(*retryableError); ok { | ||
| err = e.wrapped | ||
| } | ||
| return err | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.