Fix/s3 abandoned chunk download (#12616)

* filer: cancel a chunk download that no reader waits for

* filer: stop waiting for reader cache budget once cancelled

* filer: keep a cancelled download's re-lookups off its context

The refresh and retry lookups took the download's context. FilerClient
coalesces concurrent lookups of a volume, so a cancelled download leading
one failed the other readers waiting on it. Fetches still stop on cancel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* filer: test that cancelling a download leaves its re-lookup running

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* filer: cancel unheld downloads when the reader cache is destroyed

ReaderCache.destroy waited for every downloader, so closing a reader
waited for a stalled prefetch nobody read. Downloads with no reader or
stream on them are now cancelled first; ones still in use keep running.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* filer: test that destroying the reader cache cancels an unread prefetch

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* filer: cancel a destroyed download once its last holder leaves

destroy skipped downloads a reader or stream still held, and once they
were out of the map nothing cancelled them, so destroy waited for a fetch
nobody used. removeConsumedLocked now cancels any consumable download,
whether or not it is still in the map.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* filer: test a destroyed download cancelled after its last stream leaves

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* filer: release the stream only after destroy dropped the download

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* filer: test a cancelled stream leaving a waiting reader's download running

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
authored and GitHub committed 2026-11-13 12:45:34 -08:00
1 parent 813ac74d79
commit d695d101b5
4 files changed
+515 -18

No files matched your search

+57
View File
@@ -428,3 +428,60 @@ func TestReaderCacheReReadAfterEviction(t *testing.T) {
t.Fatalf("fetchCount=%d, want 3 (a, b, a-re-read)", got)
}
}
// A download still waiting for budget is cancelled when its last stream leaves.
func TestReaderCacheBudgetWaitStopsWithItsLastStream(t *testing.T) {
budget := NewReaderCacheBudget(4 << 10)
var lookups sync.Map
rc := NewReaderCache(256, newMockChunkCacheForReaderCache(), func(_ context.Context, fileId string) ([]string, error) {
lookups.Store(fileId, true)
return []string{"unused"}, nil
}, nil, budget)
defer rc.destroy()
holding := make(chan struct{})
var holdingOnce sync.Once
finish := make(chan struct{})
rc.fetchChunkDataFn = func(ctx context.Context, buffer []byte, _ []string, _ []byte, _ bool, _ bool, _ int64, _ string, _ util_http.RefreshUrlsFunc) (int, error) {
holdingOnce.Do(func() { close(holding) })
select {
case <-ctx.Done():
return 0, ctx.Err()
case <-finish:
return len(buffer), nil
}
}
defer close(finish)
// "a" takes the whole budget and its download does not finish
go rc.ReadChunkAt(context.Background(), make([]byte, 1), "a", nil, false, 0, 3<<10, false)
<-holding
var stream chunkStream
ctx, cancel := context.WithCancel(context.Background())
readDone := make(chan error, 1)
go func() {
_, err := rc.readChunkAt(ctx, &stream, make([]byte, 1), "b", nil, true, 0, 3<<10, false)
readDone <- err
}()
time.Sleep(50 * time.Millisecond)
if _, looked := lookups.Load("b"); looked {
t.Fatal(`"b" did not wait for budget`)
}
cancel()
if err := <-readDone; err != context.Canceled {
t.Fatalf("cancelled read: got %v, want context.Canceled", err)
}
released := make(chan struct{})
go func() {
rc.releaseStream(&stream)
close(released)
}()
select {
case <-released:
case <-time.After(2 * time.Second):
t.Fatal("releasing the stream waited for budget nobody needs")
}
}