fix(filer): return 503 + Retry-After when remote object not cached yet (#9236)
* extend cache-not-ready handling to filer HTTP path Mirror the s3api change for the native filer HTTP handlers. When the filer GET hits a remote-only object whose cache fill hasn't completed, return 503 Service Unavailable with Retry-After: 5 instead of 500 Internal Error, and treat client disconnects as silent cancellations rather than logging them as errors. Adds an ErrCacheNotReady sentinel and a small helper used at the prepareWriteFn-error sites in ProcessRangeRequest, so the same classification (cancel / not-ready / other) applies to plain GETs, single-range, and multi-range requests. * clear Content-Range on prepareWriteFn error The single-range path sets Content-Range before calling prepareWriteFn. If prepareWriteFn fails, http.Error is about to write a fresh body for 503 or 500, but the stale Content-Range header would still go out and no longer match. Drop it alongside Content-Length in the shared helper so all current and future callers are covered. * strip success-path headers and forward NotFound on prepareWriteFn error When ProcessRangeRequest writes an error response, the previously-set success headers (Content-Disposition, ETag, Last-Modified, in addition to Content-Length/Content-Range) shouldn't ride along on the new body. With ?dl=1 a stale Content-Disposition would even cause browsers to save the error message under the object's filename. Strip them all in the shared helper. Also forward filer_pb.ErrNotFound through the cache-failure branch so a mid-cache entry deletion surfaces as 404, not as a 503 retry-loop. Permanent upstream cloud errors (403/404 from the cloud SDK) still come back as opaque wrapped strings via FetchAndWriteNeedle and remain mapped to 503; distinguishing those would need a wider refactor.
This commit is contained in:
parent
4c4d53ce23
commit
7f770b1553
@ -3,6 +3,7 @@ package weed_server
|
||||
import (
|
||||
"bufio"
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
@ -28,6 +29,7 @@ import (
|
||||
"github.com/gorilla/mux"
|
||||
"github.com/seaweedfs/seaweedfs/weed/glog"
|
||||
"github.com/seaweedfs/seaweedfs/weed/operation"
|
||||
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
|
||||
"github.com/seaweedfs/seaweedfs/weed/stats"
|
||||
"github.com/seaweedfs/seaweedfs/weed/storage/needle"
|
||||
)
|
||||
@ -39,6 +41,35 @@ var writePool = sync.Pool{New: func() interface{} {
|
||||
},
|
||||
}
|
||||
|
||||
// ErrCacheNotReady signals that a remote-only object's local cache is still
|
||||
// filling. Callers should map it to 503 + Retry-After so SDKs back off and retry.
|
||||
var ErrCacheNotReady = errors.New("remote object not cached yet")
|
||||
|
||||
// writePrepareWriteFnErr writes an HTTP response for an error from
|
||||
// prepareWriteFn, before any 2xx headers have been written. Client cancels are
|
||||
// silent; filer_pb.ErrNotFound becomes 404; ErrCacheNotReady becomes 503 +
|
||||
// Retry-After; everything else is 500. Strips headers that described the
|
||||
// success body (Content-Length / Content-Range / Content-Disposition / ETag /
|
||||
// Last-Modified) so they don't get attached to the error response.
|
||||
func writePrepareWriteFnErr(w http.ResponseWriter, err error) {
|
||||
for _, h := range []string{"Content-Length", "Content-Range", "Content-Disposition", "ETag", "Last-Modified"} {
|
||||
w.Header().Del(h)
|
||||
}
|
||||
switch {
|
||||
case errors.Is(err, context.Canceled):
|
||||
glog.V(3).Infof("ProcessRangeRequest: client disconnected: %v", err)
|
||||
case errors.Is(err, filer_pb.ErrNotFound):
|
||||
http.Error(w, err.Error(), http.StatusNotFound)
|
||||
case errors.Is(err, ErrCacheNotReady):
|
||||
glog.V(1).Infof("ProcessRangeRequest: cache not ready, returning 503: %v", err)
|
||||
w.Header().Set("Retry-After", "5")
|
||||
http.Error(w, err.Error(), http.StatusServiceUnavailable)
|
||||
default:
|
||||
glog.Errorf("ProcessRangeRequest: %v", err)
|
||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||
}
|
||||
}
|
||||
|
||||
func init() {
|
||||
serverStats = stats.NewServerStats()
|
||||
go serverStats.Start()
|
||||
@ -290,9 +321,7 @@ func ProcessRangeRequest(r *http.Request, w http.ResponseWriter, totalSize int64
|
||||
w.Header().Set("Content-Length", strconv.FormatInt(totalSize, 10))
|
||||
writeFn, err := prepareWriteFn(0, totalSize)
|
||||
if err != nil {
|
||||
glog.Errorf("ProcessRangeRequest: %v", err)
|
||||
w.Header().Del("Content-Length")
|
||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||
writePrepareWriteFnErr(w, err)
|
||||
return fmt.Errorf("ProcessRangeRequest: %w", err)
|
||||
}
|
||||
if err = writeFn(bufferedWriter); err != nil {
|
||||
@ -340,9 +369,7 @@ func ProcessRangeRequest(r *http.Request, w http.ResponseWriter, totalSize int64
|
||||
|
||||
writeFn, err := prepareWriteFn(ra.start, ra.length)
|
||||
if err != nil {
|
||||
glog.Errorf("ProcessRangeRequest range[0]: %+v err: %v", w.Header(), err)
|
||||
w.Header().Del("Content-Length")
|
||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||
writePrepareWriteFnErr(w, err)
|
||||
return fmt.Errorf("ProcessRangeRequest: %w", err)
|
||||
}
|
||||
w.WriteHeader(http.StatusPartialContent)
|
||||
@ -365,9 +392,8 @@ func ProcessRangeRequest(r *http.Request, w http.ResponseWriter, totalSize int64
|
||||
}
|
||||
writeFn, err := prepareWriteFn(ra.start, ra.length)
|
||||
if err != nil {
|
||||
glog.Errorf("ProcessRangeRequest range[%d] err: %v", i, err)
|
||||
http.Error(w, "Internal Error", http.StatusInternalServerError)
|
||||
return fmt.Errorf("ProcessRangeRequest range[%d] err: %v", i, err)
|
||||
writePrepareWriteFnErr(w, err)
|
||||
return fmt.Errorf("ProcessRangeRequest range[%d]: %w", i, err)
|
||||
}
|
||||
writeFnByRange[i] = writeFn
|
||||
}
|
||||
|
||||
@ -2,6 +2,7 @@ package weed_server
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"math"
|
||||
@ -211,8 +212,18 @@ func (fs *FilerServer) GetOrHeadHandler(w http.ResponseWriter, r *http.Request)
|
||||
Name: name,
|
||||
}); err != nil {
|
||||
stats.FilerHandlerCounter.WithLabelValues(stats.ErrorReadCache).Inc()
|
||||
glog.ErrorfCtx(ctx, "CacheRemoteObjectToLocalCluster %s: %v", entry.FullPath, err)
|
||||
return nil, fmt.Errorf("cache %s: %v", entry.FullPath, err)
|
||||
// Client disconnected: surface ctx error so caller stays silent.
|
||||
if ctxErr := ctx.Err(); ctxErr != nil {
|
||||
return nil, ctxErr
|
||||
}
|
||||
// Entry vanished mid-cache: forward NotFound so caller maps to 404,
|
||||
// not the 503 retry-loop.
|
||||
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||
return nil, err
|
||||
}
|
||||
// Cache still filling: tag with sentinel so caller maps to 503 + Retry-After.
|
||||
glog.WarningfCtx(ctx, "CacheRemoteObjectToLocalCluster %s: %v", entry.FullPath, err)
|
||||
return nil, fmt.Errorf("cache %s: %w", entry.FullPath, ErrCacheNotReady)
|
||||
} else {
|
||||
chunks = resp.Entry.GetChunks()
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user