mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-16 04:06:44 +00:00
* fix(filer/remote): keep re-cache work alive past caller cancellation (#9174) For multi-GB remote blobs, doCacheRemoteObjectToLocalCluster cannot finish before the S3 gateway's initial cache wait elapses. When it does, the gRPC ctx cancellation cascades into the filer's chunk downloads, the error path calls DeleteUncommittedChunks on every chunk already written, and the next retry starts over. boto3 splitting the GET into concurrent ranges (or any client tear-down on first failure) shortens the window between retries, so the loop never converges. Detach the caller's ctx with context.WithoutCancel before invoking the singleflight work so the download runs to completion regardless of client cancellations. Subsequent waiters — via the in-flight singleflight, or a fresh retry landing after completion — observe the cached entry and stream normally. Same detach pattern is used in filer_server_handlers_write.go:53 and volume_server_handlers_write.go:51. * simplify rationale comment * switch to DoChan so handler can return on caller cancel Do keeps the handler goroutine blocked for the full detached download even after the client is gone. DoChan lets the handler select on ctx.Done() and exit immediately; the singleflight goroutine continues on bgCtx and the next request either joins it or finds the entry cached.
This commit is contained in:
@@ -21,30 +21,44 @@ import (
|
||||
)
|
||||
|
||||
func (fs *FilerServer) CacheRemoteObjectToLocalCluster(ctx context.Context, req *filer_pb.CacheRemoteObjectToLocalClusterRequest) (*filer_pb.CacheRemoteObjectToLocalClusterResponse, error) {
|
||||
// Use singleflight to deduplicate concurrent caching requests for the same object
|
||||
// Use singleflight to deduplicate concurrent caching requests for the same object.
|
||||
// This benefits all clients: S3 API, filer HTTP, Hadoop, etc.
|
||||
cacheKey := req.Directory + "/" + req.Name
|
||||
|
||||
result, err, shared := fs.remoteCacheGroup.Do(cacheKey, func() (interface{}, error) {
|
||||
return fs.doCacheRemoteObjectToLocalCluster(ctx, req)
|
||||
// Detach from caller ctx: on failure the error path deletes every chunk
|
||||
// already written, so cancelling mid-download loses all progress. For
|
||||
// blobs large enough that the download outlasts the caller's timeout
|
||||
// the retry loop never converges.
|
||||
bgCtx := context.WithoutCancel(ctx)
|
||||
|
||||
// DoChan (vs Do) so the caller can bail out on ctx.Done() while the
|
||||
// singleflight goroutine keeps caching on bgCtx; otherwise this handler
|
||||
// goroutine stays blocked for the full download after the client is gone.
|
||||
ch := fs.remoteCacheGroup.DoChan(cacheKey, func() (interface{}, error) {
|
||||
return fs.doCacheRemoteObjectToLocalCluster(bgCtx, req)
|
||||
})
|
||||
|
||||
if shared {
|
||||
glog.V(2).Infof("CacheRemoteObjectToLocalCluster: shared result for %s", cacheKey)
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
// Caller gave up; the detached cache keeps running and a later
|
||||
// request will find the entry cached (or join the same singleflight).
|
||||
return nil, ctx.Err()
|
||||
case res := <-ch:
|
||||
if res.Shared {
|
||||
glog.V(2).Infof("CacheRemoteObjectToLocalCluster: shared result for %s", cacheKey)
|
||||
}
|
||||
if res.Err != nil {
|
||||
return nil, res.Err
|
||||
}
|
||||
if res.Val == nil {
|
||||
return nil, fmt.Errorf("unexpected nil result from singleflight")
|
||||
}
|
||||
resp, ok := res.Val.(*filer_pb.CacheRemoteObjectToLocalClusterResponse)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("unexpected result type from singleflight")
|
||||
}
|
||||
return resp, nil
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if result == nil {
|
||||
return nil, fmt.Errorf("unexpected nil result from singleflight")
|
||||
}
|
||||
|
||||
resp, ok := result.(*filer_pb.CacheRemoteObjectToLocalClusterResponse)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("unexpected result type from singleflight")
|
||||
}
|
||||
return resp, nil
|
||||
}
|
||||
|
||||
// doCacheRemoteObjectToLocalCluster performs the actual caching operation.
|
||||
|
||||
Reference in New Issue
Block a user