From 1f15e7865a76ebbaee2726df34bc5534d6217940 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sat, 3 Oct 2026 12:34:55 +0800 Subject: [PATCH] filer: keep compressed chunks on the whole-chunk path Like ciphered chunks, a range request on a compressed chunk makes the volume server read and decompress the whole needle, so range-per-buffer would repeat the full backend read for each 256KiB window. Route them through the shared whole-chunk path via ChunkView.CanRangeFetch. --- weed/filer/filechunks.go | 9 +++++++++ weed/filer/reader_at.go | 10 +++++----- weed/filer/reader_at_shared_test.go | 24 +++++++++++++++++------- weed/filer/reader_cache.go | 6 +++--- 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/weed/filer/filechunks.go b/weed/filer/filechunks.go index 8cd8bb8436..9545472963 100644 --- a/weed/filer/filechunks.go +++ b/weed/filer/filechunks.go @@ -185,6 +185,15 @@ func (cv *ChunkView) IsFullChunk() bool { return cv.OffsetInChunk == 0 && cv.ViewSize == cv.ChunkSize } +// CanRangeFetch reports whether fetching just the view's byte range avoids +// reading more than the view needs. Ciphered and compressed chunks are +// stored and served whole — a range on either still costs a full read plus +// decrypt or decompress on the volume server — so partial views of them +// take the shared whole-chunk path instead. +func (cv *ChunkView) CanRangeFetch() bool { + return cv.CipherKey == nil && !cv.IsGzipped +} + func ViewFromChunks(ctx context.Context, lookupFileIdFn wdclient.LookupFileIdFunctionType, chunks []*filer_pb.FileChunk, offset int64, size int64) (chunkViews *IntervalList[*ChunkView]) { visibles, _ := NonOverlappingVisibleIntervals(ctx, lookupFileIdFn, chunks, offset, offset+size) diff --git a/weed/filer/reader_at.go b/weed/filer/reader_at.go index 4e64aeeee4..5acf09ca09 100644 --- a/weed/filer/reader_at.go +++ b/weed/filer/reader_at.go @@ -353,10 +353,10 @@ func (c *ChunkReadAt) readChunkSliceAt(ctx context.Context, buffer []byte, chunk // whose views ViewFromVisibleIntervals clips to the request) only ever // needs that part: fetch it as a range no matter the detected pattern. // Fetching the chunk whole would multiply volume-server reads. Ciphered - // chunks are the exception: the wire fetch is always the whole encrypted - // blob, so they take the shared whole-chunk path where one download - // serves every buffer. - if chunkView.CipherKey == nil && (!chunkView.IsFullChunk() || c.readerPattern.IsRandomMode()) { + // and compressed chunks are the exception: the volume server reads the + // whole blob to serve a range, so they take the shared whole-chunk path + // where one download serves every buffer. + if chunkView.CanRangeFetch() && (!chunkView.IsFullChunk() || c.readerPattern.IsRandomMode()) { c.readerCache.releaseStream(&c.stream) n, err := c.readerCache.chunkCache.ReadChunkAt(buffer, chunkView.FileId, offset) if n > 0 { @@ -386,7 +386,7 @@ func (c *ChunkReadAt) readChunkSliceAt(ctx context.Context, buffer []byte, chunk // readChunkSliceAtForParallel is a simplified version for parallel chunk fetching // It doesn't update lastChunkFid or trigger prefetch (handled by the caller) func (c *ChunkReadAt) readChunkSliceAtForParallel(ctx context.Context, buffer []byte, chunkView *ChunkView, offset uint64) (n int, err error) { - if chunkView.CipherKey == nil && !chunkView.IsFullChunk() { + if chunkView.CanRangeFetch() && !chunkView.IsFullChunk() { n, err = c.readerCache.chunkCache.ReadChunkAt(buffer, chunkView.FileId, offset) if n > 0 { return n, err diff --git a/weed/filer/reader_at_shared_test.go b/weed/filer/reader_at_shared_test.go index d4621179d2..946b669139 100644 --- a/weed/filer/reader_at_shared_test.go +++ b/weed/filer/reader_at_shared_test.go @@ -248,8 +248,9 @@ func TestChunkReadAtClippedViewsFetchOnlyCoveredParts(t *testing.T) { defer rc.destroy() fetches := fetchRecorder(rc) - // Window [56KiB, 144KiB): tail of chunk0, all of chunk1, head of - // chunk2, head of ciphered chunk3 (file chunks need not be aligned). + // Window [56KiB, 152KiB): tail of chunk0, all of chunk1, head of + // chunk2, head of ciphered chunk3, head of compressed chunk4 (file + // chunks need not be aligned). views := NewIntervalList[*ChunkView]() views.AppendInterval(&Interval[*ChunkView]{ StartOffset: chunkSize - 8<<10, @@ -271,18 +272,25 @@ func TestChunkReadAtClippedViewsFetchOnlyCoveredParts(t *testing.T) { StopOffset: 2*chunkSize + 16<<10, Value: &ChunkView{FileId: "chunk3", ViewSize: 8 << 10, ViewOffset: 2*chunkSize + 8<<10, ChunkSize: chunkSize, CipherKey: []byte("key")}, }) + views.AppendInterval(&Interval[*ChunkView]{ + StartOffset: 2*chunkSize + 16<<10, + StopOffset: 2*chunkSize + 24<<10, + Value: &ChunkView{FileId: "chunk4", ViewSize: 8 << 10, ViewOffset: 2*chunkSize + 16<<10, ChunkSize: chunkSize, IsGzipped: true}, + }) reader := NewChunkReaderAtFromClient(context.Background(), rc, views, 4*chunkSize, 0) - buf := make([]byte, chunkSize+24<<10) + buf := make([]byte, chunkSize+32<<10) if n, err := reader.ReadAt(buf, chunkSize-8<<10); err != nil || n != len(buf) { t.Fatalf("window read: n=%d err=%v", n, err) } - // buf holds [56KiB, 144KiB): chunk0's tail, chunk1, chunk2's and - // chunk3's heads. + // buf holds [56KiB, 152KiB): chunk0's tail, chunk1, and the heads of + // chunk2, chunk3 and chunk4. for i, b := range buf { want := byte('1') if i < 8<<10 { want = '0' + } else if i >= 24<<10+chunkSize { + want = '4' } else if i >= 16<<10+chunkSize { want = '3' } else if i >= 8<<10+chunkSize { @@ -297,9 +305,11 @@ func TestChunkReadAtClippedViewsFetchOnlyCoveredParts(t *testing.T) { {fileId: "chunk0", isFullChunk: false, offset: chunkSize - 8<<10, size: 8 << 10}, {fileId: "chunk1", isFullChunk: true, offset: 0, size: chunkSize}, {fileId: "chunk2", isFullChunk: false, offset: 0, size: 8 << 10}, - // partial view, but a ciphered chunk downloads whole either way and - // the shared path decrypts once for every buffer + // partial views, but ciphered and compressed chunks download whole + // either way and the shared path decrypts/decompresses once for + // every buffer {fileId: "chunk3", isFullChunk: true, offset: 0, size: chunkSize}, + {fileId: "chunk4", isFullChunk: true, offset: 0, size: chunkSize}, } got := map[string]recordedFetch{} for _, f := range *fetches { diff --git a/weed/filer/reader_cache.go b/weed/filer/reader_cache.go index 545b30375d..fb43465932 100644 --- a/weed/filer/reader_cache.go +++ b/weed/filer/reader_cache.go @@ -100,11 +100,11 @@ func (rc *ReaderCache) MaybeCache(chunkViews *Interval[*ChunkView], count int) { // abort when slots are filled return } - if chunkView.CipherKey == nil && !chunkView.IsFullChunk() { + if chunkView.CanRangeFetch() && !chunkView.IsFullChunk() { // the view is clipped to part of the chunk and will be // range-fetched, so prefetching it whole would download bytes - // nobody needs; a ciphered partial view needs the whole blob - // anyway and is worth prefetching + // nobody needs; a ciphered or compressed partial view needs + // the whole blob anyway and is worth prefetching continue }