mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-12 06:48:45 +01:00
s3, filer: bound the wait for an upload slot (#12559)
* util: add WaitUntil, a cond wait bounded by a context and a timeout The caller holds the cond lock. Cancellation and the timeout wake the waiter through a broadcast taken under the same lock, so no wakeup is lost. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3: bound the wait for an upload slot A PUT that arrived while the concurrent upload limits were exhausted waited on the condition variable with no bound. The handler reads nothing from the body while it waits, so a client that gave up and reset its connection went unnoticed: the socket and goroutine stayed pinned until a slot freed, and a stalled volume server grew the gateway's fd count by one per aborted PUT. The wait now ends on InflightUploadDataTimeout with 503 SlowDown, which S3 SDKs retry, and on request cancellation. An admitted upload takes its slot under the lock, and a finished upload broadcasts, so freed bytes admit every waiter that fits instead of one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * filer: bound the wait for an upload slot The filer's PUT/POST upload limits queued requests the same unbounded way. Reuse the bounded wait: reject with 429 after InflightUploadDataTimeout, as the volume server does, and stop on request cancellation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * command: add -inflightUploadDataTimeout for the S3 gateway and the filer weed s3 -inflightUploadDataTimeout, weed filer -inflightUploadDataTimeout and -s3.inflightUploadDataTimeout, weed server and weed mini -filer.inflightUploadDataTimeout and -s3.inflightUploadDataTimeout. The default of 60s matches the volume server's flag of the same name; 0 keeps the old unbounded wait. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3: reject a queued upload without reading its body WriteErrorResponse drains the request body before answering. Behind the stats recorder the drain has no read deadline, and reading the body sends an Expect: 100-continue client the go-ahead to upload what is about to be discarded. Answer the rejection with an empty-body copy of the request so the drain is skipped; net/http closes the connection when a large unread body remains. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3, filer: release an upload slot under the cond lock A waiter checks the counters under the cond lock and then waits. A release that decremented and broadcast without the lock could land between the two, so the waiter slept with a slot free until the next release or its timeout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3, filer: close the HTTP/1 connection when rejecting a queued upload net/http reads up to 256 KiB of an unread request body before sending a keep-alive reply, so a client that stalls its body held back the 503 or 429 until the connection idle timeout. Connection: close skips that read. HTTP/2 is left alone: there it would shut down the whole connection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
d659228338
commit
fcfca3fee3
12 files changed
+470
-50
No files matched your search
@@ -81,6 +81,7 @@ type FilerOption struct {
|
||||
SaveToFilerLimit int64
|
||||
ConcurrentUploadLimit int64
|
||||
ConcurrentFileUploadLimit int64
|
||||
InflightUploadDataTimeout time.Duration
|
||||
ShowUIDirectoryDelete bool
|
||||
DownloadMaxBytesPs int64
|
||||
DiskType string
|
||||
|
||||
@@ -3,6 +3,7 @@ package weed_server
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"path"
|
||||
"strconv"
|
||||
@@ -15,6 +16,7 @@ import (
|
||||
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
|
||||
"github.com/seaweedfs/seaweedfs/weed/security"
|
||||
"github.com/seaweedfs/seaweedfs/weed/stats"
|
||||
"github.com/seaweedfs/seaweedfs/weed/util"
|
||||
util_http "github.com/seaweedfs/seaweedfs/weed/util/http"
|
||||
"github.com/seaweedfs/seaweedfs/weed/util/version"
|
||||
)
|
||||
@@ -96,38 +98,19 @@ func (fs *FilerServer) filerHandler(w http.ResponseWriter, r *http.Request) {
|
||||
case http.MethodPost, http.MethodPut:
|
||||
// wait until in flight data is less than the limit
|
||||
contentLength := getContentLength(r)
|
||||
fs.inFlightDataLimitCond.L.Lock()
|
||||
inFlightDataSize := atomic.LoadInt64(&fs.inFlightDataSize)
|
||||
inFlightUploads := atomic.LoadInt64(&fs.inFlightUploads)
|
||||
|
||||
// Wait if either data size limit or file count limit is exceeded
|
||||
for (fs.option.ConcurrentUploadLimit != 0 && inFlightDataSize > fs.option.ConcurrentUploadLimit) || (fs.option.ConcurrentFileUploadLimit != 0 && inFlightUploads >= fs.option.ConcurrentFileUploadLimit) {
|
||||
if fs.option.ConcurrentUploadLimit != 0 && inFlightDataSize > fs.option.ConcurrentUploadLimit {
|
||||
glog.V(4).Infof("wait because inflight data %d > %d", inFlightDataSize, fs.option.ConcurrentUploadLimit)
|
||||
}
|
||||
if fs.option.ConcurrentFileUploadLimit != 0 && inFlightUploads >= fs.option.ConcurrentFileUploadLimit {
|
||||
glog.V(4).Infof("wait because inflight uploads %d >= %d", inFlightUploads, fs.option.ConcurrentFileUploadLimit)
|
||||
}
|
||||
fs.inFlightDataLimitCond.Wait()
|
||||
inFlightDataSize = atomic.LoadInt64(&fs.inFlightDataSize)
|
||||
inFlightUploads = atomic.LoadInt64(&fs.inFlightUploads)
|
||||
if !fs.waitForUploadSlot(w, r, contentLength) {
|
||||
return
|
||||
}
|
||||
fs.inFlightDataLimitCond.L.Unlock()
|
||||
|
||||
// Increment counters
|
||||
newUploads := atomic.AddInt64(&fs.inFlightUploads, 1)
|
||||
newSize := atomic.AddInt64(&fs.inFlightDataSize, contentLength)
|
||||
// Update metrics
|
||||
stats.FilerInFlightUploadCountGauge.Set(float64(newUploads))
|
||||
stats.FilerInFlightUploadBytesGauge.Set(float64(newSize))
|
||||
defer func() {
|
||||
fs.inFlightDataLimitCond.L.Lock()
|
||||
defer fs.inFlightDataLimitCond.L.Unlock()
|
||||
// Decrement counters
|
||||
newUploads := atomic.AddInt64(&fs.inFlightUploads, -1)
|
||||
newSize := atomic.AddInt64(&fs.inFlightDataSize, -contentLength)
|
||||
// Update metrics
|
||||
stats.FilerInFlightUploadCountGauge.Set(float64(newUploads))
|
||||
stats.FilerInFlightUploadBytesGauge.Set(float64(newSize))
|
||||
fs.inFlightDataLimitCond.Signal()
|
||||
fs.inFlightDataLimitCond.Broadcast()
|
||||
}()
|
||||
|
||||
if r.Method == http.MethodPut {
|
||||
@@ -145,6 +128,50 @@ func (fs *FilerServer) filerHandler(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
}
|
||||
|
||||
func (fs *FilerServer) waitForUploadSlot(w http.ResponseWriter, r *http.Request, contentLength int64) bool {
|
||||
fs.inFlightDataLimitCond.L.Lock()
|
||||
err := util.WaitUntil(r.Context(), fs.inFlightDataLimitCond, fs.option.InflightUploadDataTimeout, func() bool {
|
||||
inFlightDataSize := atomic.LoadInt64(&fs.inFlightDataSize)
|
||||
if fs.option.ConcurrentUploadLimit != 0 && inFlightDataSize > fs.option.ConcurrentUploadLimit {
|
||||
glog.V(4).Infof("wait because inflight data %d > %d", inFlightDataSize, fs.option.ConcurrentUploadLimit)
|
||||
return false
|
||||
}
|
||||
inFlightUploads := atomic.LoadInt64(&fs.inFlightUploads)
|
||||
if fs.option.ConcurrentFileUploadLimit != 0 && inFlightUploads >= fs.option.ConcurrentFileUploadLimit {
|
||||
glog.V(4).Infof("wait because inflight uploads %d >= %d", inFlightUploads, fs.option.ConcurrentFileUploadLimit)
|
||||
return false
|
||||
}
|
||||
return true
|
||||
})
|
||||
if err == nil {
|
||||
// Increment counters
|
||||
newUploads := atomic.AddInt64(&fs.inFlightUploads, 1)
|
||||
newSize := atomic.AddInt64(&fs.inFlightDataSize, contentLength)
|
||||
// Update metrics
|
||||
stats.FilerInFlightUploadCountGauge.Set(float64(newUploads))
|
||||
stats.FilerInFlightUploadBytesGauge.Set(float64(newSize))
|
||||
}
|
||||
fs.inFlightDataLimitCond.L.Unlock()
|
||||
|
||||
if err == nil {
|
||||
return true
|
||||
}
|
||||
// Closing an HTTP/1 connection stops net/http from reading the unread
|
||||
// body before the reply.
|
||||
if r.ProtoMajor == 1 {
|
||||
w.Header().Set("Connection", "close")
|
||||
}
|
||||
if r.Context().Err() != nil {
|
||||
glog.V(1).Infof("upload from %s cancelled while waiting for an upload slot: %v", r.RemoteAddr, err)
|
||||
writeJsonError(w, r, util.HttpStatusCancelled, r.Context().Err())
|
||||
return false
|
||||
}
|
||||
err = fmt.Errorf("reject because no upload slot freed within %v", fs.option.InflightUploadDataTimeout)
|
||||
glog.V(1).Infof("too many requests from %s: %v", r.RemoteAddr, err)
|
||||
writeJsonError(w, r, http.StatusTooManyRequests, err)
|
||||
return false
|
||||
}
|
||||
|
||||
func (fs *FilerServer) readonlyFilerHandler(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
start := time.Now()
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
package weed_server
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"context"
|
||||
"io"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/util"
|
||||
)
|
||||
|
||||
func filerPutRequest(ctx context.Context) *http.Request {
|
||||
return httptest.NewRequest(http.MethodPut, "/dir/file", strings.NewReader("data")).WithContext(ctx)
|
||||
}
|
||||
|
||||
func TestFilerWaitForUploadSlot(t *testing.T) {
|
||||
newFiler := func(timeout time.Duration) *FilerServer {
|
||||
fs := &FilerServer{
|
||||
option: &FilerOption{ConcurrentFileUploadLimit: 1, InflightUploadDataTimeout: timeout},
|
||||
inFlightDataLimitCond: sync.NewCond(new(sync.Mutex)),
|
||||
}
|
||||
if !fs.waitForUploadSlot(httptest.NewRecorder(), filerPutRequest(context.Background()), 4) {
|
||||
t.Fatal("first upload was not admitted")
|
||||
}
|
||||
return fs
|
||||
}
|
||||
|
||||
t.Run("times out with 429", func(t *testing.T) {
|
||||
fs := newFiler(100 * time.Millisecond)
|
||||
rec := httptest.NewRecorder()
|
||||
start := time.Now()
|
||||
if fs.waitForUploadSlot(rec, filerPutRequest(context.Background()), 4) {
|
||||
t.Fatal("upload admitted without a free slot")
|
||||
}
|
||||
if rec.Code != http.StatusTooManyRequests {
|
||||
t.Fatalf("got %d, want 429", rec.Code)
|
||||
}
|
||||
if elapsed := time.Since(start); elapsed < 100*time.Millisecond {
|
||||
t.Fatalf("rejected after %v, before the wait timeout", elapsed)
|
||||
}
|
||||
if fs.inFlightUploads != 1 || fs.inFlightDataSize != 4 {
|
||||
t.Fatalf("in-flight uploads %d bytes %d, want 1 and 4", fs.inFlightUploads, fs.inFlightDataSize)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("stalled body still gets the 429", func(t *testing.T) {
|
||||
fs := newFiler(100 * time.Millisecond)
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if fs.waitForUploadSlot(w, r, r.ContentLength) {
|
||||
t.Error("upload admitted without a free slot")
|
||||
}
|
||||
}))
|
||||
defer srv.Close()
|
||||
conn, err := net.Dial("tcp", srv.Listener.Addr().String())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer conn.Close()
|
||||
conn.SetDeadline(time.Now().Add(3 * time.Second))
|
||||
if _, err := io.WriteString(conn, "PUT /dir/file HTTP/1.1\r\nHost: filer\r\nContent-Length: 10\r\n\r\n"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
resp, err := http.ReadResponse(bufio.NewReader(conn), nil)
|
||||
if err != nil {
|
||||
t.Fatalf("no reply while the body is unsent: %v", err)
|
||||
}
|
||||
if resp.StatusCode != http.StatusTooManyRequests || !resp.Close {
|
||||
t.Fatalf("got %d close=%v, want 429 with Connection: close", resp.StatusCode, resp.Close)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("cancelled request stops waiting", func(t *testing.T) {
|
||||
fs := newFiler(0)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
time.AfterFunc(50*time.Millisecond, cancel)
|
||||
done := make(chan int)
|
||||
go func() {
|
||||
rec := httptest.NewRecorder()
|
||||
if fs.waitForUploadSlot(rec, filerPutRequest(ctx), 4) {
|
||||
t.Error("cancelled upload admitted")
|
||||
}
|
||||
done <- rec.Code
|
||||
}()
|
||||
select {
|
||||
case code := <-done:
|
||||
if code != util.HttpStatusCancelled {
|
||||
t.Fatalf("got %d, want %d", code, util.HttpStatusCancelled)
|
||||
}
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("cancelled upload kept waiting for a slot")
|
||||
}
|
||||
})
|
||||
}
|
||||
Reference in new issue
Block a user