diff --git a/weed/filer/filer_deletion.go b/weed/filer/filer_deletion.go index d8bc105e6..6d472d0a7 100644 --- a/weed/filer/filer_deletion.go +++ b/weed/filer/filer_deletion.go @@ -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) { diff --git a/weed/replication/sink/filersink/fetch_write.go b/weed/replication/sink/filersink/fetch_write.go index 845427c5c..b8532f091 100644 --- a/weed/replication/sink/filersink/fetch_write.go +++ b/weed/replication/sink/filersink/fetch_write.go @@ -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. diff --git a/weed/util/retry.go b/weed/util/retry.go index a8f7ffb41..3cee2ce1e 100644 --- a/weed/util/retry.go +++ b/weed/util/retry.go @@ -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) { diff --git a/weed/util/retry_test.go b/weed/util/retry_test.go index 099293545..f96cd8236 100644 --- a/weed/util/retry_test.go +++ b/weed/util/retry_test.go @@ -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 { diff --git a/weed/wdclient/filer_client.go b/weed/wdclient/filer_client.go index 5c3b35089..65237fb3c 100644 --- a/weed/wdclient/filer_client.go +++ b/weed/wdclient/filer_client.go @@ -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.