diff --git a/weed/filer/filechunk_group.go b/weed/filer/filechunk_group.go index 9bba17b6e..6cc45a041 100644 --- a/weed/filer/filechunk_group.go +++ b/weed/filer/filechunk_group.go @@ -20,6 +20,10 @@ type ChunkGroup struct { concurrentReaders int // cacheInvalidator lets manifest resolution drop stale volume locations, as ReaderCache does for chunk reads cacheInvalidator CacheInvalidator + // resolveErr is set when chunk manifest resolution failed, guarded by + // sectionsLock. Reads must fail with this error instead of silently + // zero-filling the unresolved sections as if they were sparse holes. + resolveErr error } // NewChunkGroup creates a ChunkGroup with configurable concurrency. @@ -91,6 +95,13 @@ func (group *ChunkGroup) ReadDataAt(ctx context.Context, fileSize int64, buff [] group.sectionsLock.RLock() defer group.sectionsLock.RUnlock() + // Fail fast when chunk manifest resolution failed: the sections map is + // empty or partial, and zero-filling it would silently return all-zero + // data as if the file were one big sparse hole. + if group.resolveErr != nil { + return 0, 0, group.resolveErr + } + sectionIndexStart, sectionIndexStop := SectionIndex(offset/SectionSize), SectionIndex((offset+int64(len(buff)))/SectionSize) numSections := int(sectionIndexStop - sectionIndexStart + 1) @@ -232,6 +243,9 @@ func (group *ChunkGroup) SetChunks(chunks []*filer_pb.FileChunk) error { resolvedChunks, err := ResolveOneChunkManifest(context.Background(), group.lookupFn, chunk, group.cacheInvalidator) if err != nil { + // remember the failure so ReadDataAt returns an error instead of + // treating the unresolved sections as sparse holes + group.resolveErr = err return err } @@ -253,6 +267,7 @@ func (group *ChunkGroup) SetChunks(chunks []*filer_pb.FileChunk) error { } group.sections = sections + group.resolveErr = nil return nil } @@ -262,11 +277,17 @@ const ( // SEEK_HOLE uint32 = 4 // seek to next hole after the offset ) -func (group *ChunkGroup) SearchChunks(ctx context.Context, offset, fileSize int64, whence uint32) (found bool, out int64) { +func (group *ChunkGroup) SearchChunks(ctx context.Context, offset, fileSize int64, whence uint32) (found bool, out int64, err error) { group.sectionsLock.RLock() defer group.sectionsLock.RUnlock() - return group.doSearchChunks(ctx, offset, fileSize, whence) + // the section map is unreliable after a failed manifest resolution + if group.resolveErr != nil { + return false, 0, group.resolveErr + } + + found, out = group.doSearchChunks(ctx, offset, fileSize, whence) + return found, out, nil } func (group *ChunkGroup) doSearchChunks(ctx context.Context, offset, fileSize int64, whence uint32) (found bool, out int64) { diff --git a/weed/filer/filechunk_group_test.go b/weed/filer/filechunk_group_test.go index b04a61c83..4926f2809 100644 --- a/weed/filer/filechunk_group_test.go +++ b/weed/filer/filechunk_group_test.go @@ -196,7 +196,7 @@ func TestChunkGroup_SearchChunks_Cancellation(t *testing.T) { whence := uint32(3) // SEEK_DATA // Call SearchChunks with cancelled context - found, resultOffset := group.SearchChunks(ctx, offset, fileSize, whence) + found, resultOffset, _ := group.SearchChunks(ctx, offset, fileSize, whence) // For an empty ChunkGroup, SearchChunks should complete quickly // The main goal is to verify the context parameter is properly threaded through @@ -225,7 +225,7 @@ func TestChunkGroup_SearchChunks_Cancellation(t *testing.T) { whence := uint32(3) // SEEK_DATA // Call SearchChunks - should complete quickly for empty group - found, resultOffset := group.SearchChunks(ctx, offset, fileSize, whence) + found, resultOffset, _ := group.SearchChunks(ctx, offset, fileSize, whence) // Verify reasonable behavior assert.False(t, found, "should not find data in empty chunk group") @@ -426,9 +426,48 @@ func TestChunkGroup_SearchChunks(t *testing.T) { return } - gotFound, gotOffset := group.SearchChunks(context.Background(), tt.args.offset, tt.args.fileSize, tt.args.whence) + gotFound, gotOffset, err := group.SearchChunks(context.Background(), tt.args.offset, tt.args.fileSize, tt.args.whence) + assert.NoError(t, err) assert.Equalf(t, tt.wantFound, gotFound, "SearchChunks(%v, %v, %v) found", tt.args.offset, tt.args.fileSize, tt.args.whence) assert.Equalf(t, tt.wantOffset, gotOffset, "SearchChunks(%v, %v, %v) offset", tt.args.offset, tt.args.fileSize, tt.args.whence) }) } } + +// Regression test for silent zero-fill reads when chunk manifest resolution +// fails: ReadDataAt must return an error instead of treating the unresolved +// sections as sparse holes (https://github.com/seaweedfs/seaweedfs/issues/11286). +func TestChunkGroup_ReadDataAt_ManifestResolveFailure(t *testing.T) { + lookupErr := errors.New("lookup failed") + lookupFn := func(ctx context.Context, fileId string) ([]string, error) { + return nil, lookupErr + } + + chunks := []*filer_pb.FileChunk{ + {FileId: "1,1679011dc64abd40", IsChunkManifest: true, Offset: 0, Size: 1 << 20}, + } + + group, err := NewChunkGroup(lookupFn, nil, chunks, 1, nil) + assert.Error(t, err, "manifest resolution should fail") + + // Reads must fail with the resolve error, not silently return zeros. + buff := make([]byte, 16) + n, _, readErr := group.ReadDataAt(context.Background(), 1<<20, buff, 0) + assert.ErrorIs(t, readErr, lookupErr) + assert.Equal(t, 0, n) + + // lseek (SEEK_DATA/SEEK_HOLE) must fail too, not misreport the whole + // file as sparse. + for _, whence := range []uint32{SEEK_DATA, 4 /* SEEK_HOLE */} { + found, _, seekErr := group.SearchChunks(context.Background(), 0, 1<<20, whence) + assert.ErrorIs(t, seekErr, lookupErr, "whence %d", whence) + assert.False(t, found, "whence %d", whence) + } + + // A later successful SetChunks must clear the error. + err = group.SetChunks([]*filer_pb.FileChunk{ + {FileId: "2,data", Offset: 0, Size: 16}, + }) + assert.NoError(t, err) + assert.NoError(t, group.resolveErr) +} diff --git a/weed/filer/filechunks.go b/weed/filer/filechunks.go index 8cd8bb843..1102e2fe0 100644 --- a/weed/filer/filechunks.go +++ b/weed/filer/filechunks.go @@ -193,6 +193,19 @@ func ViewFromChunks(ctx context.Context, lookupFileIdFn wdclient.LookupFileIdFun } +// viewFromChunksOrErr is ViewFromChunks with the manifest resolve error +// propagated. Ignoring it yields empty chunk views and the caller zero-fills +// the whole requested range (https://github.com/seaweedfs/seaweedfs/issues/11286). +func viewFromChunksOrErr(ctx context.Context, lookupFileIdFn wdclient.LookupFileIdFunctionType, chunks []*filer_pb.FileChunk, offset int64, size int64) (*IntervalList[*ChunkView], error) { + + visibles, err := NonOverlappingVisibleIntervals(ctx, lookupFileIdFn, chunks, offset, offset+size) + if err != nil { + return nil, err + } + + return ViewFromVisibleIntervals(visibles, offset, size), nil +} + func ViewFromVisibleIntervals(visibles *IntervalList[*VisibleInterval], offset int64, size int64) (chunkViews *IntervalList[*ChunkView]) { stop := offset + size diff --git a/weed/filer/stream.go b/weed/filer/stream.go index 8642339c4..dcb66d765 100644 --- a/weed/filer/stream.go +++ b/weed/filer/stream.go @@ -171,7 +171,10 @@ func retryFetchWithFreshLocations(ctx context.Context, invalidator CacheInvalida func PrepareStreamContentWithThrottler(ctx context.Context, masterClient wdclient.HasLookupFileIdFunction, jwtFunc VolumeServerJwtFunction, chunks []*filer_pb.FileChunk, offset int64, size int64, downloadMaxBytesPs int64) (DoStreamContent, error) { glog.V(4).InfofCtx(ctx, "prepare to stream content for chunks: %d", len(chunks)) - chunkViews := ViewFromChunks(ctx, masterClient.GetLookupFileIdFunction(), chunks, offset, size) + chunkViews, err := viewFromChunksOrErr(ctx, masterClient.GetLookupFileIdFunction(), chunks, offset, size) + if err != nil { + return nil, err + } fileId2Url := make(map[string][]string) @@ -285,7 +288,10 @@ func PrepareStreamContentWithPrefetch(ctx context.Context, masterClient wdclient } glog.V(4).InfofCtx(ctx, "prepare to stream content with prefetch=%d for chunks: %d", prefetchAhead, len(chunks)) - chunkViews := ViewFromChunks(ctx, masterClient.GetLookupFileIdFunction(), chunks, offset, size) + chunkViews, err := viewFromChunksOrErr(ctx, masterClient.GetLookupFileIdFunction(), chunks, offset, size) + if err != nil { + return nil, err + } fileId2Url := make(map[string][]string) diff --git a/weed/filer/stream_manifest_error_test.go b/weed/filer/stream_manifest_error_test.go new file mode 100644 index 000000000..349559db5 --- /dev/null +++ b/weed/filer/stream_manifest_error_test.go @@ -0,0 +1,27 @@ +package filer + +import ( + "context" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/stretchr/testify/assert" +) + +// Regression test for #11286: stream preparation must fail when chunk +// manifest resolution fails, instead of serving a zero-filled stream. +func TestPrepareStreamContent_ManifestResolveFailure(t *testing.T) { + master := &testMasterClient{} // no urls registered: every lookup fails + + chunks := []*filer_pb.FileChunk{ + {FileId: "1,1879011dc64abd40", IsChunkManifest: true, Offset: 0, Size: 1 << 20}, + } + + _, err := PrepareStreamContentWithThrottler(context.Background(), master, noopJwt, chunks, 0, 1<<20, 0) + assert.Error(t, err) + assert.Contains(t, err.Error(), "fail to read manifest") + + _, err = PrepareStreamContentWithPrefetch(context.Background(), master, noopJwt, chunks, 0, 1<<20, 0, 4) + assert.Error(t, err) + assert.Contains(t, err.Error(), "fail to read manifest") +} diff --git a/weed/mount/weedfs_file_lseek.go b/weed/mount/weedfs_file_lseek.go index 826c1e0fc..06ebfc144 100644 --- a/weed/mount/weedfs_file_lseek.go +++ b/weed/mount/weedfs_file_lseek.go @@ -70,7 +70,11 @@ func (wfs *WFS) Lseek(cancel <-chan struct{}, in *fuse.LseekIn, out *fuse.LseekO }() // search chunks for the offset - found, offset := fh.entryChunkGroup.SearchChunks(ctx, offset, fileSize, in.Whence) + found, offset, err := fh.entryChunkGroup.SearchChunks(ctx, offset, fileSize, in.Whence) + if err != nil { + glog.Errorf("Lseek %s: %v", fh.FullPath(), err) + return fuse.EIO + } if found { out.Offset = uint64(offset) return fuse.OK diff --git a/weed/server/webdav_server.go b/weed/server/webdav_server.go index 7acac1d2f..88d64a431 100644 --- a/weed/server/webdav_server.go +++ b/weed/server/webdav_server.go @@ -568,7 +568,11 @@ func (f *WebDavFile) Read(p []byte) (readSize int, err error) { return 0, io.EOF } if f.visibleIntervals == nil { - f.visibleIntervals, _ = filer.NonOverlappingVisibleIntervals(f.ctx, f.fs.filerClient.GetLookupFileIdFunction(), f.entry.GetChunks(), 0, fileSize) + f.visibleIntervals, err = filer.NonOverlappingVisibleIntervals(f.ctx, f.fs.filerClient.GetLookupFileIdFunction(), f.entry.GetChunks(), 0, fileSize) + if err != nil { + // fail instead of streaming zeros for unresolved manifest chunks + return 0, err + } f.reader = nil } if f.reader == nil {