mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-16 04:06:44 +00:00
consolidate the duplicated transient-error classifiers onto util.IsTransientError (#10429)
* util: match transient error messages case-insensitively, expose the message form The same condition reaches different layers capitalized differently -- a volume server relays its idle timeout as "I/O timeout" inside a JSON string -- and the callers that grew their own substring lists all lower-case first. Also split out IsTransientErrorMessage for the paths that carry only the text, such as the per-file status strings in a batch delete response, and pick up "no route to host" and "network is unreachable" from the gRPC classifier. * filersink: classify transient network errors through util.IsTransientError The local list caught i/o timeout, connection reset, and broken pipe but not connection refused, no such host, unexpected EOF, the syscall errnos, or the gRPC and S3 overload codes. Keep only the bare io.EOF case, which is transient here -- a truncated chunk read -- but a clean stream end elsewhere. * filer deletion: reuse util.IsTransientErrorMessage for the network patterns Six of the sixteen patterns were already covered. Keep the ones specific to this pipeline -- read-only volumes, lookup failures, backpressure -- and note why context cancellation stays retryable here: it decides whether to requeue the deletion, not whether to retry a call. * wdclient: fold the shared classifier into the volume lookup retry check The string tail duplicated the shared list and missed the syscall errnos and net.Error timeouts. Keep "connection" and "timeout", which are broader than the shared classifier on purpose: a volume lookup is a cheap read-only call.
This commit is contained in:
@@ -40,25 +40,24 @@ const (
|
||||
DeletionBatchSize = 100000
|
||||
)
|
||||
|
||||
// retryablePatterns contains error message patterns that indicate temporary/transient conditions
|
||||
// that should be retried. These patterns are based on actual error messages from the deletion pipeline.
|
||||
// retryablePatterns contains transient conditions specific to the deletion
|
||||
// pipeline, on top of the network and service failures util.IsTransientErrorMessage
|
||||
// already covers.
|
||||
//
|
||||
// Context cancellation counts as retryable here but not in util: this decides
|
||||
// whether to requeue the deletion, not whether to retry a call, and a cancelled
|
||||
// batch leaves the file still needing deletion.
|
||||
var retryablePatterns = []string{
|
||||
"is read only", // Volume temporarily read-only (tiering, maintenance)
|
||||
"error reading from server", // Network I/O errors
|
||||
"connection reset by peer", // Network connection issues
|
||||
"closed network connection", // Network connection closed unexpectedly
|
||||
"connection refused", // Server temporarily unavailable
|
||||
"timeout", // Operation timeout (network or server)
|
||||
"deadline exceeded", // Context deadline exceeded
|
||||
"context canceled", // Context cancellation (may be transient)
|
||||
"context canceled", // Context cancellation
|
||||
"lookup error", // Volume lookup failures
|
||||
"lookup failed", // Volume server discovery issues
|
||||
"too many requests", // Rate limiting / backpressure
|
||||
"service unavailable", // HTTP 503 errors
|
||||
"temporarily unavailable", // Temporary service issues
|
||||
"try again", // Explicit retry suggestion
|
||||
"i/o timeout", // Network I/O timeout
|
||||
"broken pipe", // Connection broken during operation
|
||||
}
|
||||
|
||||
// DeletionRetryItem represents a file deletion that failed and needs to be retried
|
||||
@@ -491,6 +490,10 @@ func isRetryableError(errorMsg string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
if util.IsTransientErrorMessage(errorMsg) {
|
||||
return true
|
||||
}
|
||||
|
||||
errorLower := strings.ToLower(errorMsg)
|
||||
for _, pattern := range retryablePatterns {
|
||||
if strings.Contains(errorLower, pattern) {
|
||||
|
||||
@@ -6,7 +6,6 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net"
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -465,25 +464,14 @@ func isEofError(err error) bool {
|
||||
}
|
||||
|
||||
// isRetryableNetworkError reports whether err is a transient network failure worth
|
||||
// a backoff-and-retry: EOF, timeout (e.g. the destination's idle deadline under
|
||||
// load), or a reset/broken connection. The volume server returns the timeout as a
|
||||
// JSON string, so match on text as well as the net.Error interface.
|
||||
// a backoff-and-retry. A bare io.EOF counts here but not in util.IsTransientError:
|
||||
// mid-transfer it means the chunk read was truncated, not that a stream ended
|
||||
// cleanly.
|
||||
func isRetryableNetworkError(err error) bool {
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
if isEofError(err) {
|
||||
return true
|
||||
}
|
||||
var netErr net.Error
|
||||
if errors.As(err, &netErr) && netErr.Timeout() {
|
||||
return true
|
||||
}
|
||||
// lower-cased to also catch capitalized variants
|
||||
msg := strings.ToLower(err.Error())
|
||||
return strings.Contains(msg, "i/o timeout") ||
|
||||
strings.Contains(msg, "connection reset") ||
|
||||
strings.Contains(msg, "broken pipe")
|
||||
return isEofError(err) || util.IsTransientError(err)
|
||||
}
|
||||
|
||||
// errChunkSizeMismatch is a permanent (non-retriable) replication failure.
|
||||
|
||||
+30
-13
@@ -18,25 +18,42 @@ var RetryWaitTime = 6 * time.Second
|
||||
// likely to get past: connection resets, timeouts, and the throttling or
|
||||
// overload replies S3 and gRPC hand back. Cloud SDKs bury the underlying net
|
||||
// error in an opaque wrapper with no Unwrap, so the message is often all
|
||||
// that is left to match on.
|
||||
// that is left to match on. Compared case-insensitively, so entries are lower
|
||||
// case: the same condition reaches different layers capitalized differently
|
||||
// (a volume server relays its idle timeout as "I/O timeout" inside JSON).
|
||||
var transientErrorMessages = []string{
|
||||
"transport",
|
||||
"connection reset",
|
||||
"connection refused",
|
||||
"broken pipe",
|
||||
"unexpected EOF",
|
||||
"unexpected eof",
|
||||
"i/o timeout",
|
||||
"TLS handshake timeout",
|
||||
"tls handshake timeout",
|
||||
"no such host",
|
||||
"Client.Timeout",
|
||||
"RequestError",
|
||||
"RequestTimeout",
|
||||
"SlowDown",
|
||||
"Throttling",
|
||||
"InternalError",
|
||||
"ServiceUnavailable",
|
||||
"ResourceExhausted",
|
||||
"Unavailable",
|
||||
"no route to host",
|
||||
"network is unreachable",
|
||||
"client.timeout",
|
||||
"requesterror",
|
||||
"requesttimeout",
|
||||
"slowdown",
|
||||
"throttling",
|
||||
"internalerror",
|
||||
"resourceexhausted",
|
||||
"unavailable",
|
||||
}
|
||||
|
||||
// IsTransientErrorMessage reports whether an error message describes a network
|
||||
// or service condition worth retrying. Callers holding an error should use
|
||||
// IsTransientError; this is for paths that only carry the text, such as the
|
||||
// per-file status strings in a batch delete response.
|
||||
func IsTransientErrorMessage(msg string) bool {
|
||||
lower := strings.ToLower(msg)
|
||||
for _, transient := range transientErrorMessages {
|
||||
if strings.Contains(lower, transient) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// IsTransientError reports whether err is a network or service condition worth
|
||||
@@ -58,7 +75,7 @@ func IsTransientError(err error) bool {
|
||||
if errors.As(err, &netErr) && netErr.Timeout() {
|
||||
return true
|
||||
}
|
||||
return containErr(err.Error(), transientErrorMessages)
|
||||
return IsTransientErrorMessage(err.Error())
|
||||
}
|
||||
|
||||
func Retry(name string, job func() error) (err error) {
|
||||
|
||||
@@ -43,6 +43,34 @@ func TestIsTransientError(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsTransientErrorMessage(t *testing.T) {
|
||||
transient := []string{
|
||||
"read tcp 10.0.0.1:8082->10.0.0.1:54848: i/o timeout",
|
||||
// the same condition relayed by a volume server inside a JSON string
|
||||
"Upload result: read tcp 10.0.0.1:8082->10.0.0.1:54848: I/O timeout",
|
||||
"Connection reset by peer",
|
||||
"dial tcp 10.0.0.1:8888: connect: no route to host",
|
||||
"rpc error: code = Unavailable desc = the connection is unavailable",
|
||||
}
|
||||
for _, msg := range transient {
|
||||
if !IsTransientErrorMessage(msg) {
|
||||
t.Errorf("expected transient: %q", msg)
|
||||
}
|
||||
}
|
||||
|
||||
permanent := []string{
|
||||
"",
|
||||
"not found",
|
||||
"invalid file id",
|
||||
"chunk size mismatch",
|
||||
}
|
||||
for _, msg := range permanent {
|
||||
if IsTransientErrorMessage(msg) {
|
||||
t.Errorf("expected permanent: %q", msg)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRetryTransientError(t *testing.T) {
|
||||
callCount := 0
|
||||
err := Retry("test", func() error {
|
||||
|
||||
@@ -17,6 +17,7 @@ import (
|
||||
"github.com/seaweedfs/seaweedfs/weed/glog"
|
||||
"github.com/seaweedfs/seaweedfs/weed/pb"
|
||||
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
|
||||
"github.com/seaweedfs/seaweedfs/weed/util"
|
||||
)
|
||||
|
||||
// UrlPreference controls which URL to use for volume access
|
||||
@@ -519,12 +520,13 @@ func isRetryableGrpcError(err error) bool {
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback to string matching for non-gRPC errors (e.g., network errors)
|
||||
errStr := err.Error()
|
||||
return strings.Contains(errStr, "transport") ||
|
||||
// Fallback for non-gRPC errors (e.g. network errors). "connection" and
|
||||
// "timeout" are deliberately broader than the shared classifier: a volume
|
||||
// lookup is a cheap read-only call, so leaning towards a retry is fine.
|
||||
errStr := strings.ToLower(err.Error())
|
||||
return util.IsTransientError(err) ||
|
||||
strings.Contains(errStr, "connection") ||
|
||||
strings.Contains(errStr, "timeout") ||
|
||||
strings.Contains(errStr, "unavailable")
|
||||
strings.Contains(errStr, "timeout")
|
||||
}
|
||||
|
||||
// jitter returns a duration in the range [d/2, d) using equal jitter.
|
||||
|
||||
Reference in New Issue
Block a user