diff --git a/weed/mount/metadata_flush_retry_test.go b/weed/mount/metadata_flush_retry_test.go index 7bf3a23ff..c72032f24 100644 --- a/weed/mount/metadata_flush_retry_test.go +++ b/weed/mount/metadata_flush_retry_test.go @@ -81,10 +81,10 @@ func TestRetryMetadataFlushReturnsLastError(t *testing.T) { } } -// TestRetryMetadataFlushStopsOnCancel verifies that an interrupted flush (the -// calling process was killed, so the FUSE cancel channel fired) abandons its -// retries immediately instead of sleeping out the backoff, so the killed -// process is not held in close() while the filer is overwhelmed. +// TestRetryMetadataFlushStopsOnCancel verifies that once the flush context is +// done (the flush deadline elapsed against an overwhelmed filer) the retry loop +// abandons its retries immediately instead of sleeping out the backoff, so +// close() is not held longer than the deadline. func TestRetryMetadataFlushStopsOnCancel(t *testing.T) { originalSleep := metadataFlushSleep t.Cleanup(func() { @@ -97,7 +97,7 @@ func TestRetryMetadataFlushStopsOnCancel(t *testing.T) { } ctx, cancel := context.WithCancel(context.Background()) - cancel() // process already killed before the first attempt completes + cancel() // flush deadline already elapsed before the first attempt completes attempts := 0 flushErr := errors.New("filer overwhelmed") diff --git a/weed/mount/weedfs_file_sync.go b/weed/mount/weedfs_file_sync.go index 0082178a2..487e6d03d 100644 --- a/weed/mount/weedfs_file_sync.go +++ b/weed/mount/weedfs_file_sync.go @@ -16,6 +16,12 @@ import ( "google.golang.org/protobuf/proto" ) +// metadataFlushTimeout bounds a close()/fsync metadata flush so an overwhelmed +// filer cannot wedge the calling process forever. It is deliberately generous: +// a healthy CreateEntry completes in well under a second, so this only fires on +// a genuinely stuck filer, never on a normal flush. +const metadataFlushTimeout = 30 * time.Second + /** * Flush method * @@ -72,10 +78,14 @@ func (wfs *WFS) Flush(cancel <-chan struct{}, in *fuse.FlushIn) fuse.Status { hasPosixLocks := wfs.hasPosixOwner(in.NodeId, in.LockOwner) allowAsync := !hasPosixLocks - // Abort the flush when the kernel interrupts the request (the calling - // process was killed); otherwise close() hangs in uninterruptible sleep - // while the metadata flush retries against an overwhelmed filer. - ctx, cancelFunc := fuseInterruptContext(cancel) + // Bound the flush with a deadline instead of tying it to the FUSE cancel + // channel. A FUSE interrupt is not a process kill: Go's async preemption + // (SIGURG) makes a close() under load emit an interrupt on nearly every + // flush (see go-fuse RawFileSystem docs), so cancelling the in-flight + // metadata CreateEntry on that interrupt turned healthy concurrent close()s + // into EIO. The deadline still keeps close() from hanging forever against an + // overwhelmed filer without failing benign flushes. + ctx, cancelFunc := context.WithTimeout(context.Background(), metadataFlushTimeout) defer cancelFunc() status := wfs.doFlush(ctx, fh, in.Uid, in.Gid, allowAsync) @@ -85,22 +95,6 @@ func (wfs *WFS) Flush(cancel <-chan struct{}, in *fuse.FlushIn) fuse.Status { return status } -// fuseInterruptContext returns a context cancelled when the FUSE cancel channel -// fires (request interrupted). The caller must call the returned func. -func fuseInterruptContext(cancel <-chan struct{}) (context.Context, context.CancelFunc) { - ctx, cancelFunc := context.WithCancel(context.Background()) - if cancel != nil { - go func() { - select { - case <-cancel: - cancelFunc() - case <-ctx.Done(): - } - }() - } - return ctx, cancelFunc -} - /** * Synchronize file contents * @@ -127,7 +121,7 @@ func (wfs *WFS) Fsync(cancel <-chan struct{}, in *fuse.FsyncIn) (code fuse.Statu return fuse.ENOENT } - ctx, cancelFunc := fuseInterruptContext(cancel) + ctx, cancelFunc := context.WithTimeout(context.Background(), metadataFlushTimeout) defer cancelFunc() // Fsync is an explicit sync request — always flush synchronously diff --git a/weed/mount/weedfs_file_sync_test.go b/weed/mount/weedfs_file_sync_test.go index 044115646..ac8c51872 100644 --- a/weed/mount/weedfs_file_sync_test.go +++ b/weed/mount/weedfs_file_sync_test.go @@ -6,7 +6,6 @@ import ( "math" "math/rand/v2" "testing" - "time" "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" @@ -443,48 +442,3 @@ func TestVisibleContentPreservedAfterCompact(t *testing.T) { } } } - -// TestFuseInterruptContextCancelsOnInterrupt verifies the interrupt wiring: -// when the kernel interrupts a flush/fsync (the calling process was killed) -// go-fuse closes the cancel channel, and the derived context must be cancelled -// so the in-flight metadata RPC and its retries abort instead of leaving the -// killed process stuck in uninterruptible sleep inside close(). -func TestFuseInterruptContextCancelsOnInterrupt(t *testing.T) { - cancel := make(chan struct{}) - ctx, cancelFunc := fuseInterruptContext(cancel) - defer cancelFunc() - - select { - case <-ctx.Done(): - t.Fatal("context cancelled before the FUSE request was interrupted") - default: - } - - close(cancel) // kernel sent FUSE_INTERRUPT (process killed) - - select { - case <-ctx.Done(): - case <-time.After(5 * time.Second): - t.Fatal("context not cancelled after FUSE interrupt") - } -} - -// A nil cancel channel (no interrupt plumbing) must still yield a usable, -// cancellable context that does not fire on its own. -func TestFuseInterruptContextNilChannel(t *testing.T) { - ctx, cancelFunc := fuseInterruptContext(nil) - defer cancelFunc() - - select { - case <-ctx.Done(): - t.Fatal("context cancelled with no interrupt and no explicit cancel") - default: - } - - cancelFunc() - select { - case <-ctx.Done(): - case <-time.After(5 * time.Second): - t.Fatal("context not cancelled after explicit cancelFunc") - } -}