Browse Source

Filer: Modernize heap interface and improve error handling docs

1. Replace interface{} with any in heap methods
   - Addresses modern Go style (Go 1.18+)
   - Improves code readability

2. Enhance isRetryableError documentation
   - Acknowledge string matching brittleness
   - Add comprehensive TODO for future improvements:
     * Use HTTP status codes (503, 429, etc.)
     * Implement structured error types with errors.Is/As
     * Extract gRPC status codes
     * Add error wrapping for better context
   - Document each error pattern with context
   - Add defensive check for empty error strings

Current implementation remains pragmatic for initial release while
documenting a clear path for future robustness improvements. String
matching is acceptable for now but should be replaced with structured
error checking when refactoring the deletion pipeline.
pull/7402/head
Dimonyga 1 month ago
parent
commit
066a588742
  1. 35
      weed/filer/filer_deletion.go

35
weed/filer/filer_deletion.go

@ -55,13 +55,13 @@ func (h retryHeap) Swap(i, j int) {
h[j].heapIndex = j h[j].heapIndex = j
} }
func (h *retryHeap) Push(x interface{}) {
func (h *retryHeap) Push(x any) {
item := x.(*DeletionRetryItem) item := x.(*DeletionRetryItem)
item.heapIndex = len(*h) item.heapIndex = len(*h)
*h = append(*h, item) *h = append(*h, item)
} }
func (h *retryHeap) Pop() interface{} {
func (h *retryHeap) Pop() any {
old := *h old := *h
n := len(old) n := len(old)
item := old[n-1] item := old[n-1]
@ -269,14 +269,33 @@ func (f *Filer) loopProcessingDeletion() {
} }
} }
// isRetryableError determines if an error is retryable
// isRetryableError determines if an error is retryable based on its message.
//
// Current implementation uses string matching which is brittle and may break
// if error messages change in dependencies. This is acceptable for the initial
// implementation but should be improved in the future.
//
// TODO: Consider these improvements for more robust error handling:
// - Pass DeleteResult instead of just error string to access Status codes
// - Use HTTP status codes (503 Service Unavailable, 429 Too Many Requests, etc.)
// - Implement structured error types that can be checked with errors.Is/errors.As
// - Extract and check gRPC status codes for better classification
// - Add error wrapping in the deletion pipeline to preserve error context
//
// For now, we use conservative string matching for known transient error patterns.
func isRetryableError(errorMsg string) bool { func isRetryableError(errorMsg string) bool {
// Errors that indicate temporary conditions
// Empty errors are not retryable
if errorMsg == "" {
return false
}
// Known patterns that indicate temporary/transient conditions.
// These are based on actual error messages from the deletion pipeline.
retryablePatterns := []string{ retryablePatterns := []string{
"is read only",
"error reading from server",
"connection reset by peer",
"closed network connection",
"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
} }
errorLower := strings.ToLower(errorMsg) errorLower := strings.ToLower(errorMsg)

Loading…
Cancel
Save