mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-08 15:41:15 +02:00
mount: fix deadlock reading an uncached remote-mounted file (#9995)
* mount: apply cached remote entry without blocking the read Reading an uncached remote-mounted file hung forever. The read holds the file-handle shared lock across the on-demand download, then synchronously waits on the metadata apply loop to apply the cached entry. The filer's update event for that same object reaches the apply loop first and runs invalidateFunc, which wants the file-handle exclusive lock — held in shared mode by the still-running read. The loop blocks on the read; the read blocks on the loop. Enqueue the apply without waiting so the read never blocks on the apply loop while holding the lock. The handle is already updated via SetEntry, and the filer subscription delivers the same event regardless. * mount: regression test for uncached remote read deadlock Drives the real Read path through downloadRemoteEntry with a stub filer that broadcasts the matching invalidate event before returning, reproducing the lock-ordering deadlock. Fails (times out) without the fix.
This commit is contained in:
@@ -195,13 +195,12 @@ func (fh *FileHandle) downloadRemoteEntry(entry *LockedEntry) error {
|
||||
|
||||
fh.SetEntry(resp.Entry)
|
||||
|
||||
// Async: a sync apply deadlocks against the apply loop's invalidate, which needs this read's file-handle lock.
|
||||
event := resp.GetMetadataEvent()
|
||||
if event == nil {
|
||||
event = metadataUpdateEvent(request.Directory, resp.Entry)
|
||||
}
|
||||
if applyErr := fh.wfs.applyLocalMetadataEvent(context.Background(), event); applyErr != nil {
|
||||
glog.Warningf("CacheRemoteObject %s: best-effort metadata apply failed: %v", fileFullPath, applyErr)
|
||||
}
|
||||
fh.wfs.applyLocalMetadataEventAsync(event)
|
||||
|
||||
return nil
|
||||
})
|
||||
|
||||
@@ -0,0 +1,149 @@
|
||||
package mount
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/seaweedfs/go-fuse/v2/fuse"
|
||||
"github.com/seaweedfs/seaweedfs/weed/mount/meta_cache"
|
||||
"github.com/seaweedfs/seaweedfs/weed/pb"
|
||||
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
|
||||
"github.com/seaweedfs/seaweedfs/weed/util"
|
||||
"google.golang.org/grpc"
|
||||
"google.golang.org/grpc/credentials/insecure"
|
||||
)
|
||||
|
||||
// cacheRemoteTestServer broadcasts the matching invalidate event before
|
||||
// returning the cached entry, so the apply loop is busy invalidating the same
|
||||
// file handle the read still holds when downloadRemoteEntry applies its event.
|
||||
type cacheRemoteTestServer struct {
|
||||
filer_pb.UnimplementedSeaweedFilerServer
|
||||
wfs *WFS
|
||||
dir, name string
|
||||
content []byte
|
||||
invalidateStarted chan struct{}
|
||||
}
|
||||
|
||||
func (s *cacheRemoteTestServer) CacheRemoteObjectToLocalCluster(ctx context.Context, req *filer_pb.CacheRemoteObjectToLocalClusterRequest) (*filer_pb.CacheRemoteObjectToLocalClusterResponse, error) {
|
||||
cached := &filer_pb.Entry{
|
||||
Name: s.name,
|
||||
Attributes: &filer_pb.FuseAttributes{FileSize: uint64(len(s.content))},
|
||||
Content: s.content,
|
||||
}
|
||||
go s.wfs.metaCache.ApplyMetadataResponse(context.Background(), metadataUpdateEvent(s.dir, cached), meta_cache.SubscriberMetadataResponseApplyOptions)
|
||||
<-s.invalidateStarted
|
||||
return &filer_pb.CacheRemoteObjectToLocalClusterResponse{
|
||||
Entry: cached,
|
||||
MetadataEvent: metadataUpdateEvent(s.dir, cached),
|
||||
}, nil
|
||||
}
|
||||
|
||||
// TestReadUncachedRemoteEntryDoesNotDeadlock guards the read of an uncached
|
||||
// remote file against the apply-loop invalidate that needs its file-handle lock.
|
||||
func TestReadUncachedRemoteEntryDoesNotDeadlock(t *testing.T) {
|
||||
listener, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
if err != nil {
|
||||
t.Fatalf("listen: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = listener.Close() })
|
||||
|
||||
server := pb.NewGrpcServer()
|
||||
testServer := &cacheRemoteTestServer{
|
||||
dir: "/dir",
|
||||
name: "file",
|
||||
content: []byte("hello remote world"),
|
||||
invalidateStarted: make(chan struct{}, 1),
|
||||
}
|
||||
filer_pb.RegisterSeaweedFilerServer(server, testServer)
|
||||
go server.Serve(listener)
|
||||
t.Cleanup(server.Stop)
|
||||
|
||||
uidGidMapper, err := meta_cache.NewUidGidMapper("", "")
|
||||
if err != nil {
|
||||
t.Fatalf("create uid/gid mapper: %v", err)
|
||||
}
|
||||
|
||||
root := util.FullPath("/")
|
||||
wfs := &WFS{
|
||||
signature: 1,
|
||||
inodeToPath: NewInodeToPath(root, 0),
|
||||
fhMap: NewFileHandleToInode(),
|
||||
fhLockTable: util.NewLockTable[FileHandleId](),
|
||||
hardLinkLockTable: util.NewLockTable[string](),
|
||||
option: &Option{
|
||||
ChunkSizeLimit: 1024,
|
||||
ConcurrentReaders: 1,
|
||||
VolumeServerAccess: "filerProxy",
|
||||
FilerAddresses: []pb.ServerAddress{
|
||||
pb.NewServerAddressWithGrpcPort("127.0.0.1:1", listener.Addr().(*net.TCPAddr).Port),
|
||||
},
|
||||
GrpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
|
||||
},
|
||||
}
|
||||
testServer.wfs = wfs
|
||||
|
||||
wfs.metaCache = meta_cache.NewMetaCache(
|
||||
filepath.Join(t.TempDir(), "meta"),
|
||||
uidGidMapper,
|
||||
root,
|
||||
false,
|
||||
func(path util.FullPath) { wfs.inodeToPath.MarkChildrenCached(path) },
|
||||
func(path util.FullPath) bool { return wfs.inodeToPath.IsChildrenCached(path) },
|
||||
// Mirror weedfs.go's invalidateFunc: take the file handle exclusive lock.
|
||||
func(path util.FullPath, _ *filer_pb.Entry) {
|
||||
inode, ok := wfs.inodeToPath.GetInode(path)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
fh, ok := wfs.fhMap.FindFileHandle(inode)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
select {
|
||||
case testServer.invalidateStarted <- struct{}{}:
|
||||
default:
|
||||
}
|
||||
lock := wfs.fhLockTable.AcquireLock("invalidateFunc", fh.fh, util.ExclusiveLock)
|
||||
wfs.fhLockTable.ReleaseLock(fh.fh, lock)
|
||||
},
|
||||
nil,
|
||||
)
|
||||
wfs.inodeToPath.MarkChildrenCached(root)
|
||||
wfs.inodeToPath.Lookup(util.FullPath("/dir"), time.Now().Unix(), true, false, 0, false)
|
||||
wfs.inodeToPath.MarkChildrenCached(util.FullPath("/dir"))
|
||||
t.Cleanup(func() { wfs.metaCache.Shutdown() })
|
||||
|
||||
inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false)
|
||||
fh := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{
|
||||
Name: "file",
|
||||
Attributes: &filer_pb.FuseAttributes{FileSize: uint64(len(testServer.content))},
|
||||
RemoteEntry: &filer_pb.RemoteEntry{RemoteSize: int64(len(testServer.content))},
|
||||
})
|
||||
|
||||
buff := make([]byte, len(testServer.content))
|
||||
done := make(chan fuse.Status, 1)
|
||||
go func() {
|
||||
_, status := wfs.Read(make(chan struct{}), &fuse.ReadIn{
|
||||
InHeader: fuse.InHeader{NodeId: inode},
|
||||
Fh: uint64(fh.fh),
|
||||
Offset: 0,
|
||||
Size: uint32(len(buff)),
|
||||
}, buff)
|
||||
done <- status
|
||||
}()
|
||||
|
||||
select {
|
||||
case status := <-done:
|
||||
if status != fuse.OK {
|
||||
t.Fatalf("Read status = %v, want OK", status)
|
||||
}
|
||||
if string(buff) != string(testServer.content) {
|
||||
t.Fatalf("Read content = %q, want %q", buff, testServer.content)
|
||||
}
|
||||
case <-time.After(15 * time.Second):
|
||||
t.Fatal("Read of an uncached remote entry deadlocked")
|
||||
}
|
||||
}
|
||||
@@ -34,7 +34,7 @@ type MetaCache struct {
|
||||
invalidateFunc func(fullpath util.FullPath, entry *filer_pb.Entry)
|
||||
onDirectoryUpdate func(dir util.FullPath)
|
||||
pinnedChildFn func(*filer.Entry) bool // a child a rebuild must not drop (local-only, not yet on the filer); nil disables
|
||||
visitGroup singleflight.Group // deduplicates concurrent EnsureVisited calls for the same path
|
||||
visitGroup singleflight.Group // deduplicates concurrent EnsureVisited calls for the same path
|
||||
applyCh chan metadataApplyRequest
|
||||
applyDone chan struct{}
|
||||
applyStateMu sync.Mutex
|
||||
@@ -228,6 +228,25 @@ func (mc *MetaCache) ApplyMetadataResponseOwned(ctx context.Context, resp *filer
|
||||
return mc.applyMetadataResponseEnqueue(ctx, resp, options)
|
||||
}
|
||||
|
||||
// ApplyMetadataResponseOwnedAsync enqueues resp without waiting, for callers holding a
|
||||
// lock the apply loop's invalidateFunc also needs. Best-effort: the subscription re-delivers.
|
||||
func (mc *MetaCache) ApplyMetadataResponseOwnedAsync(resp *filer_pb.SubscribeMetadataResponse, options MetadataResponseApplyOptions) {
|
||||
if resp == nil || resp.EventNotification == nil {
|
||||
return
|
||||
}
|
||||
req := metadataApplyRequest{
|
||||
ctx: context.Background(),
|
||||
kind: metadataApplyEvent,
|
||||
resp: resp,
|
||||
options: options,
|
||||
done: make(chan error, 1),
|
||||
}
|
||||
select {
|
||||
case mc.applyCh <- req:
|
||||
default:
|
||||
}
|
||||
}
|
||||
|
||||
func (mc *MetaCache) applyMetadataResponseEnqueue(ctx context.Context, resp *filer_pb.SubscribeMetadataResponse, options MetadataResponseApplyOptions) error {
|
||||
if ctx == nil {
|
||||
ctx = context.Background()
|
||||
|
||||
@@ -15,6 +15,10 @@ func (wfs *WFS) applyLocalMetadataEvent(ctx context.Context, event *filer_pb.Sub
|
||||
return wfs.metaCache.ApplyMetadataResponseOwned(ctx, event, meta_cache.LocalMetadataResponseApplyOptions)
|
||||
}
|
||||
|
||||
func (wfs *WFS) applyLocalMetadataEventAsync(event *filer_pb.SubscribeMetadataResponse) {
|
||||
wfs.metaCache.ApplyMetadataResponseOwnedAsync(event, meta_cache.LocalMetadataResponseApplyOptions)
|
||||
}
|
||||
|
||||
func metadataDeleteEvent(directory, name string, isDirectory bool) *filer_pb.SubscribeMetadataResponse {
|
||||
if name == "" {
|
||||
return nil
|
||||
|
||||
Reference in New Issue
Block a user