mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-11 22:38:53 +01:00
mount: keep the shared directory build out of the requester's context (#12206)
* mount: keep the shared directory build out of the requester's context The shared directory build behind EnsureVisited is a singleflight call: the first requester for a path owns the build, and every later requester for the same path waits on that same build. The build ran on the owner's request context, so a FUSE request that returned -- or that the kernel interrupted -- canceled the build for everybody waiting on it. A waiter whose own context was still alive then received somebody else's cancellation. weed/mount/weedfs_dir_read.go classified a canceled read as EINTR only when the local context was done, so such a waiter fell through to fuse.EIO: the kernel handed EIO to readdirent, kubelet's applyFSGroup failed, and the volume never mounted. Build the shared directory on a context that keeps the request's values but not its cancellation, and report a canceled read as EINTR even when the local context is still alive. Waiters still abandon on their own context, so a canceled request stops waiting immediately. This follows up #11717, which made the wait abandonable and mapped the local cancellation to EINTR, but left the shared build itself on the owner's context. The surrounding build lifecycle calls already use context.Background() for the same reason. Test: TestEnsureVisitedSharedBuildSurvivesOwnerRequestCancel reproduces the cancellation with a fake filer and fails without this change. * mount: classify gRPC-canceled directory listings as EINTR * mount: log only real directory read failures, not interruptions * mount: keep shared section refresh out of the requester's context * mount: let section-refresh waiters leave on their own context The shared refresh flight now outlives the requester that started it, but callers still blocked in singleflight.Do until it finished — an interrupted directory read could not stop waiting, and the caller's own deadline never applied to the wait. DoChan + select on the caller's context restores early abandon while the detached refresh still completes for the remaining waiters. * ci: re-run FUSE integration after a flaky writeback-cache failure * mount: refresh a stale entry once before reporting a read error A read resolves chunk views from the handle's entry. A concurrent overwrite commits a new entry whose replaced file ids the filer queues for deletion; the open handle only learns the new chunk list when the metadata invalidation lands. A fetch that lands between the volume delete and the invalidation names data that is already gone and the read reports EIO. On a chunk read error, wait out any in-flight async flush, re-read the entry from the filer, install it when it outranks the handle, and retry once. An unchanged version means the failure is genuine. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: fence the read-error entry refresh by file identity Two guards on refreshEntryForRead's install path: the unlocked dirtyMetadata read raced with Write under the handle lock — drop it, installAckedEntry already checks under that lock, and it now fences isDeleted handles the same way. And a saved path can name a different file after unlink/reuse, so refuse the refresh when the fetched inode disagrees with the one the handle holds. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: user.email <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
8 files changed
+349
-47
No files matched your search
@@ -0,0 +1,180 @@
|
||||
package meta_cache
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
|
||||
"github.com/seaweedfs/seaweedfs/weed/util"
|
||||
"google.golang.org/grpc"
|
||||
"google.golang.org/grpc/metadata"
|
||||
)
|
||||
|
||||
// The shared directory build behind EnsureVisited is a singleflight call: the
|
||||
// first requester for a path owns the build, and every later requester for the
|
||||
// same path waits on that same build. If the build runs on the owner's request
|
||||
// context, the owner walking away (a FUSE request returning, the kernel
|
||||
// interrupting it) kills the build for everybody else.
|
||||
//
|
||||
// A waiter whose own context is still alive then receives somebody else's
|
||||
// cancellation. weed/mount/weedfs_dir_read.go classifies a canceled read as
|
||||
// EINTR only when the *local* context is done, so the waiter falls through to
|
||||
// fuse.EIO — the kernel hands EIO to readdirent, kubelet's applyFSGroup fails,
|
||||
// and the volume never mounts. That is the 2026-10-28 gitea outage.
|
||||
//
|
||||
// The mock below reproduces it deterministically: the filer stream blocks
|
||||
// mid-listing, the owner's context is canceled while the build is in flight,
|
||||
// and a second requester joins that same build.
|
||||
|
||||
// ctxAwareListStream models a filer listing that fails with the request
|
||||
// context's error once that context is canceled, the way a real gRPC stream
|
||||
// does.
|
||||
type ctxAwareListStream struct {
|
||||
ctx context.Context
|
||||
responses []*filer_pb.ListEntriesResponse
|
||||
entered chan struct{}
|
||||
release chan struct{}
|
||||
once sync.Once
|
||||
index int
|
||||
}
|
||||
|
||||
func (s *ctxAwareListStream) Recv() (*filer_pb.ListEntriesResponse, error) {
|
||||
s.once.Do(func() {
|
||||
close(s.entered)
|
||||
<-s.release
|
||||
})
|
||||
if err := s.ctx.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if s.index >= len(s.responses) {
|
||||
return nil, io.EOF
|
||||
}
|
||||
resp := s.responses[s.index]
|
||||
s.index++
|
||||
return resp, nil
|
||||
}
|
||||
|
||||
func (s *ctxAwareListStream) Header() (metadata.MD, error) { return metadata.MD{}, nil }
|
||||
func (s *ctxAwareListStream) Trailer() metadata.MD { return metadata.MD{} }
|
||||
func (s *ctxAwareListStream) CloseSend() error { return nil }
|
||||
func (s *ctxAwareListStream) Context() context.Context { return s.ctx }
|
||||
func (s *ctxAwareListStream) SendMsg(any) error { return nil }
|
||||
func (s *ctxAwareListStream) RecvMsg(any) error { return nil }
|
||||
|
||||
// ctxAwareListClient counts how many listings were started. The shared build
|
||||
// must be listed exactly once no matter how many requesters join it.
|
||||
type ctxAwareListClient struct {
|
||||
filer_pb.SeaweedFilerClient
|
||||
stream *ctxAwareListStream
|
||||
|
||||
mu sync.Mutex
|
||||
calls int
|
||||
}
|
||||
|
||||
func (c *ctxAwareListClient) ListEntries(ctx context.Context, in *filer_pb.ListEntriesRequest, opts ...grpc.CallOption) (grpc.ServerStreamingClient[filer_pb.ListEntriesResponse], error) {
|
||||
c.mu.Lock()
|
||||
c.calls++
|
||||
c.mu.Unlock()
|
||||
c.stream.ctx = ctx
|
||||
return c.stream, nil
|
||||
}
|
||||
|
||||
func (c *ctxAwareListClient) listCalls() int {
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
return c.calls
|
||||
}
|
||||
|
||||
func TestEnsureVisitedSharedBuildSurvivesOwnerRequestCancel(t *testing.T) {
|
||||
mc, _, _, _ := newTestMetaCache(t, map[util.FullPath]bool{
|
||||
"/": true,
|
||||
})
|
||||
defer mc.Shutdown()
|
||||
|
||||
entered := make(chan struct{})
|
||||
release := make(chan struct{})
|
||||
client := &ctxAwareListClient{
|
||||
stream: &ctxAwareListStream{
|
||||
responses: []*filer_pb.ListEntriesResponse{
|
||||
{
|
||||
Entry: &filer_pb.Entry{
|
||||
Name: "base.txt",
|
||||
Attributes: &filer_pb.FuseAttributes{
|
||||
Crtime: 1,
|
||||
Mtime: 1,
|
||||
FileMode: 0100644,
|
||||
FileSize: 3,
|
||||
},
|
||||
},
|
||||
SnapshotTsNs: 100,
|
||||
},
|
||||
},
|
||||
entered: entered,
|
||||
release: release,
|
||||
},
|
||||
}
|
||||
accessor := &buildFilerAccessor{client: client}
|
||||
|
||||
// The owner starts the shared build.
|
||||
ownerCtx, cancelOwner := context.WithCancel(context.Background())
|
||||
ownerErr := make(chan error, 1)
|
||||
go func() {
|
||||
ownerErr <- EnsureVisited(ownerCtx, mc, accessor, util.FullPath("/dir"), 0)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-entered:
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatal("shared build never reached the filer listing")
|
||||
}
|
||||
|
||||
// The owner's FUSE request goes away while the build is still in flight.
|
||||
cancelOwner()
|
||||
if err := <-ownerErr; !errors.Is(err, context.Canceled) {
|
||||
t.Fatalf("owner error = %v, want context.Canceled", err)
|
||||
}
|
||||
|
||||
// A second requester arrives and joins the still-running shared build.
|
||||
waiterErr := make(chan error, 1)
|
||||
go func() {
|
||||
waiterErr <- EnsureVisited(context.Background(), mc, accessor, util.FullPath("/dir"), 0)
|
||||
}()
|
||||
|
||||
// Give the waiter time to join. If it started its own build instead, the
|
||||
// listing is requested twice — the repro would not be the production path.
|
||||
deadline := time.Now().Add(2 * time.Second)
|
||||
for client.listCalls() < 2 && time.Now().Before(deadline) {
|
||||
select {
|
||||
case err := <-waiterErr:
|
||||
t.Fatalf("waiter returned before the shared build finished (%v); it started a second listing (%d calls) instead of joining the shared build",
|
||||
err, client.listCalls())
|
||||
case <-time.After(10 * time.Millisecond):
|
||||
}
|
||||
}
|
||||
if calls := client.listCalls(); calls != 1 {
|
||||
t.Fatalf("listing calls = %d, want 1 (the waiter must join the shared build)", calls)
|
||||
}
|
||||
|
||||
// Let the build finish.
|
||||
close(release)
|
||||
|
||||
select {
|
||||
case err := <-waiterErr:
|
||||
if err != nil {
|
||||
t.Fatalf("waiter inherited the owner's cancellation: %v", err)
|
||||
}
|
||||
case <-time.After(30 * time.Second):
|
||||
t.Fatal("waiter never returned")
|
||||
}
|
||||
|
||||
if calls := client.listCalls(); calls != 1 {
|
||||
t.Fatalf("listing calls = %d, want 1", calls)
|
||||
}
|
||||
if !mc.IsDirectoryCached(util.FullPath("/dir")) {
|
||||
t.Fatal("directory /dir should be cached after the shared build completes")
|
||||
}
|
||||
}
|
||||
@@ -91,13 +91,26 @@ const (
|
||||
)
|
||||
|
||||
func doEnsureVisited(ctx context.Context, mc *MetaCache, client filer_pb.FilerClient, path util.FullPath, maxCacheableEntries int) error {
|
||||
// The shared build must not run on the requester's context. A FUSE request
|
||||
// that returns — or that the kernel interrupts — cancels that context, and
|
||||
// every other requester waiting on the same build would inherit the
|
||||
// cancellation. weed/mount/weedfs_dir_read.go only reports EINTR when the
|
||||
// *local* context is done, so such a waiter falls through to fuse.EIO and
|
||||
// the kernel hands EIO to readdirent: kubelet's applyFSGroup fails and the
|
||||
// volume never mounts.
|
||||
//
|
||||
// Build on a context that keeps the request's values but not its
|
||||
// cancellation. Waiters still abandon on their own context below, so a
|
||||
// canceled request stops waiting immediately.
|
||||
buildCtx := context.WithoutCancel(ctx)
|
||||
|
||||
// Use singleflight to deduplicate concurrent requests for the same path.
|
||||
// DoChan so a canceled request stops waiting even when another request
|
||||
// owns the in-flight build.
|
||||
resultCh := mc.visitGroup.DoChan(string(path), func() (interface{}, error) {
|
||||
// Check for cancellation before starting
|
||||
if ctx.Err() != nil {
|
||||
return nil, ctx.Err()
|
||||
if buildCtx.Err() != nil {
|
||||
return nil, buildCtx.Err()
|
||||
}
|
||||
|
||||
// Double-check if already cached (another goroutine may have completed)
|
||||
@@ -128,7 +141,7 @@ func doEnsureVisited(ctx context.Context, mc *MetaCache, client filer_pb.FilerCl
|
||||
}
|
||||
}
|
||||
defer func() {
|
||||
if !cleanupDone && ctx.Err() != nil {
|
||||
if !cleanupDone && buildCtx.Err() != nil {
|
||||
cleanupBuild("canceled")
|
||||
}
|
||||
}()
|
||||
@@ -139,11 +152,11 @@ func doEnsureVisited(ctx context.Context, mc *MetaCache, client filer_pb.FilerCl
|
||||
entryCount = 0
|
||||
sections = sectionBoundsCollector{}
|
||||
var batch []*filer.Entry // reset on retry, allow GC of previous entries
|
||||
if err := mc.deleteFolderChildrenForRebuild(ctx, path); err != nil {
|
||||
if err := mc.deleteFolderChildrenForRebuild(buildCtx, path); err != nil {
|
||||
return fmt.Errorf("clear existing entries for %s: %w", path, err)
|
||||
}
|
||||
var listErr error
|
||||
snapshotTsNs, listErr = filer_pb.ReadDirAllEntriesWithSnapshot(ctx, client, path, "", func(pbEntry *filer_pb.Entry, isLast bool) error {
|
||||
snapshotTsNs, listErr = filer_pb.ReadDirAllEntriesWithSnapshot(buildCtx, client, path, "", func(pbEntry *filer_pb.Entry, isLast bool) error {
|
||||
entry := filer.FromPbEntry(string(path), pbEntry)
|
||||
if !mc.includeSystemEntries && IsHiddenSystemEntry(string(path), entry.Name()) {
|
||||
return nil
|
||||
@@ -158,7 +171,7 @@ func doEnsureVisited(ctx context.Context, mc *MetaCache, client filer_pb.FilerCl
|
||||
|
||||
// flush by size, not isLast: hidden entries can return early
|
||||
if len(batch) >= batchInsertSize {
|
||||
if err := mc.doBatchInsertEntries(ctx, batch); err != nil {
|
||||
if err := mc.doBatchInsertEntries(buildCtx, batch); err != nil {
|
||||
return fmt.Errorf("batch insert for %s: %w", path, err)
|
||||
}
|
||||
batch = make([]*filer.Entry, 0, batchInsertSize)
|
||||
@@ -169,7 +182,7 @@ func doEnsureVisited(ctx context.Context, mc *MetaCache, client filer_pb.FilerCl
|
||||
return listErr
|
||||
}
|
||||
if len(batch) > 0 {
|
||||
if err := mc.doBatchInsertEntries(ctx, batch); err != nil {
|
||||
if err := mc.doBatchInsertEntries(buildCtx, batch); err != nil {
|
||||
return fmt.Errorf("batch insert remaining for %s: %w", path, err)
|
||||
}
|
||||
}
|
||||
@@ -198,14 +211,14 @@ func doEnsureVisited(ctx context.Context, mc *MetaCache, client filer_pb.FilerCl
|
||||
// immediate (a clean-EOF stream glitch clears at once), later ones space out.
|
||||
// On cancellation the deferred cleanup aborts the build.
|
||||
for attempt := 0; entryCount == 0 && attempt < emptyRebuildConfirmations; attempt++ {
|
||||
if ctx.Err() != nil {
|
||||
return nil, ctx.Err()
|
||||
if buildCtx.Err() != nil {
|
||||
return nil, buildCtx.Err()
|
||||
}
|
||||
if attempt > 0 {
|
||||
select {
|
||||
case <-time.After(emptyRebuildConfirmDelay):
|
||||
case <-ctx.Done():
|
||||
return nil, ctx.Err()
|
||||
case <-buildCtx.Done():
|
||||
return nil, buildCtx.Err()
|
||||
}
|
||||
}
|
||||
if entryCount, snapshotTsNs, sections, fetchErr = reloadFromFiler(); fetchErr != nil {
|
||||
|
||||
@@ -285,21 +285,33 @@ func (mc *MetaCache) staleRangeOf(dirPath util.FullPath, name string) (nameRange
|
||||
}
|
||||
|
||||
func (mc *MetaCache) refreshSection(ctx context.Context, client filer_pb.FilerClient, dirPath util.FullPath, lo, hi string) error {
|
||||
_, err, _ := mc.visitGroup.Do(string(dirPath)+"\x00section\x00"+lo, func() (interface{}, error) {
|
||||
// DoChan so a requester that leaves mid-refresh stops waiting on its own
|
||||
// context while the shared refresh continues for the remaining waiters.
|
||||
resultCh := mc.visitGroup.DoChan(string(dirPath)+"\x00section\x00"+lo, func() (interface{}, error) {
|
||||
if !mc.rangeStale(dirPath, lo) {
|
||||
return nil, nil
|
||||
}
|
||||
entries, snapshotTsNs, err := mc.listFilerRange(ctx, client, dirPath, lo, hi)
|
||||
// Shared like the directory build: the requester that started this
|
||||
// flight can leave mid-refresh, so re-apply the bound on a context it
|
||||
// cannot cancel rather than failing every waiter on its cancellation.
|
||||
flightCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), sectionRefreshTimeout)
|
||||
defer cancel()
|
||||
entries, snapshotTsNs, err := mc.listFilerRange(flightCtx, client, dirPath, lo, hi)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return nil, mc.enqueueAndWait(ctx, metadataApplyRequest{
|
||||
return nil, mc.enqueueAndWait(flightCtx, metadataApplyRequest{
|
||||
kind: metadataSectionRefresh,
|
||||
buildPath: dirPath,
|
||||
refresh: §ionRefresh{lo: lo, hi: hi, entries: entries, snapshotTsNs: snapshotTsNs},
|
||||
})
|
||||
})
|
||||
return err
|
||||
select {
|
||||
case result := <-resultCh:
|
||||
return result.Err
|
||||
case <-ctx.Done():
|
||||
return ctx.Err()
|
||||
}
|
||||
}
|
||||
|
||||
// listFilerRange reads [lo, hi) from the filer at one snapshot, paging by
|
||||
|
||||
Reference in new issue
Block a user