From ef109fe9e155da177390b9db084a4ce70b67b399 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 24 Jun 2026 14:24:22 -0700 Subject: [PATCH] mount: don't hang close() when a writer is killed during flush (#10090) * operation: bound AssignVolume with a deadline AssignVolume ran on context.Background(), so when the filer is overwhelmed the RPC could block indefinitely and wedge every caller holding the connection. Give it a 30s deadline so a stuck assign fails and the caller's retry/error path runs instead of hanging forever. * mount: abort flush when the FUSE request is interrupted On close(), a killed process blocks in fuse_flush waiting for the mount to answer. doFlush ran its metadata CreateEntry on context.Background() and ignored the kernel interrupt channel, so against an overwhelmed filer the flush never completed and the process stayed in uninterruptible sleep -- making the pod un-killable. Derive a context from the FUSE cancel channel in Flush/Fsync and thread it through doFlush -> flushMetadataToFiler -> streamCreateEntry; the retry loop stops as soon as the context is cancelled. Release and the pre-rename flush keep a non-cancellable context since they must finish regardless. * operation: harden the AssignVolume timeout test Make the test double's signal send non-blocking and bound the receive with a timeout so a regression can't wedge the test instead of failing it. --- weed/mount/error_classifier_test.go | 9 ++-- weed/mount/metadata_flush_retry.go | 39 +++++++++----- weed/mount/metadata_flush_retry_test.go | 45 ++++++++++++++-- weed/mount/weedfs_async_flush.go | 5 +- weed/mount/weedfs_file_io.go | 6 ++- weed/mount/weedfs_file_mkrm.go | 2 +- weed/mount/weedfs_file_sync.go | 40 +++++++++++--- weed/mount/weedfs_file_sync_test.go | 50 ++++++++++++++++- weed/mount/weedfs_rename.go | 3 +- weed/mount/wfs_save.go | 2 +- weed/operation/upload_content.go | 8 ++- weed/operation/upload_content_test.go | 72 +++++++++++++++++++++++++ 12 files changed, 245 insertions(+), 36 deletions(-) diff --git a/weed/mount/error_classifier_test.go b/weed/mount/error_classifier_test.go index 1414cf2e8..b49db4f99 100644 --- a/weed/mount/error_classifier_test.go +++ b/weed/mount/error_classifier_test.go @@ -1,6 +1,7 @@ package mount import ( + "context" "errors" "fmt" "syscall" @@ -82,13 +83,13 @@ func TestRetryMetadataFlushIfShortCircuitsOnPermanentError(t *testing.T) { t.Cleanup(func() { metadataFlushSleep = originalSleep }) - metadataFlushSleep = func(_ time.Duration) { + metadataFlushSleep = func(_ context.Context, _ time.Duration) { t.Fatal("sleep should not be called when shouldRetry returns false") } attempts := 0 permanent := status.Error(codes.NotFound, "entry missing") - err := retryMetadataFlushIf(func() error { + err := retryMetadataFlushIf(context.Background(), func() error { attempts++ return permanent }, isRetryableFilerError, nil) @@ -108,11 +109,11 @@ func TestRetryMetadataFlushIfRetriesTransientErrors(t *testing.T) { t.Cleanup(func() { metadataFlushSleep = originalSleep }) - metadataFlushSleep = func(_ time.Duration) {} + metadataFlushSleep = func(_ context.Context, _ time.Duration) {} attempts := 0 transient := status.Error(codes.Canceled, "grpc: the client connection is closing") - err := retryMetadataFlushIf(func() error { + err := retryMetadataFlushIf(context.Background(), func() error { attempts++ return transient }, isRetryableFilerError, nil) diff --git a/weed/mount/metadata_flush_retry.go b/weed/mount/metadata_flush_retry.go index 5a4f3c8d8..8e14cffee 100644 --- a/weed/mount/metadata_flush_retry.go +++ b/weed/mount/metadata_flush_retry.go @@ -1,21 +1,30 @@ package mount -import "time" +import ( + "context" + "time" +) const metadataFlushRetries = 3 -var metadataFlushSleep = time.Sleep - -func retryMetadataFlush(flush func() error, onRetry func(nextAttempt, totalAttempts int, backoff time.Duration, err error)) error { - return retryMetadataFlushIf(flush, nil, onRetry) +// metadataFlushSleep waits for d or until ctx is cancelled. Overridable in tests. +var metadataFlushSleep = func(ctx context.Context, d time.Duration) { + timer := time.NewTimer(d) + defer timer.Stop() + select { + case <-ctx.Done(): + case <-timer.C: + } } -// retryMetadataFlushIf is retryMetadataFlush with an optional shouldRetry -// predicate. If shouldRetry is nil or returns true, the flush is retried with -// exponential backoff; if it returns false, the error is returned immediately -// so callers don't pay retry latency on clearly permanent errors (e.g. -// ENOENT/EACCES/EINVAL from a synchronous setattr). -func retryMetadataFlushIf(flush func() error, shouldRetry func(error) bool, onRetry func(nextAttempt, totalAttempts int, backoff time.Duration, err error)) error { +func retryMetadataFlush(ctx context.Context, flush func() error, onRetry func(nextAttempt, totalAttempts int, backoff time.Duration, err error)) error { + return retryMetadataFlushIf(ctx, flush, nil, onRetry) +} + +// retryMetadataFlushIf retries flush with exponential backoff, stopping early +// when shouldRetry returns false (clearly permanent errors) or when ctx is +// cancelled (the FUSE request was interrupted, e.g. the process was killed). +func retryMetadataFlushIf(ctx context.Context, flush func() error, shouldRetry func(error) bool, onRetry func(nextAttempt, totalAttempts int, backoff time.Duration, err error)) error { totalAttempts := metadataFlushRetries + 1 var err error for attempt := 1; attempt <= totalAttempts; attempt++ { @@ -29,12 +38,18 @@ func retryMetadataFlushIf(flush func() error, shouldRetry func(error) bool, onRe if shouldRetry != nil && !shouldRetry(err) { break } + if ctx.Err() != nil { + break + } backoff := time.Duration(1<