diff --git a/weed/storage/needle/needle_parse_upload.go b/weed/storage/needle/needle_parse_upload.go index f7f62df11..aa54c3462 100644 --- a/weed/storage/needle/needle_parse_upload.go +++ b/weed/storage/needle/needle_parse_upload.go @@ -17,6 +17,31 @@ import ( "github.com/seaweedfs/seaweedfs/weed/util" ) +// maxEagerPreGrow caps how many bytes ParseUpload is willing to pre-allocate +// from a request's announced Content-Length. Large enough to skip a few +// rounds of bytes.Buffer.ReadFrom geometric grow on typical small uploads; +// small enough that a misreported Content-Length or a slow/idle client can +// only ever waste this much memory per request. Bigger uploads fall back to +// the standard ReadFrom-grow path for the remainder. +const maxEagerPreGrow = 4 * 1024 * 1024 + +// eagerPreGrow ensures bytesBuffer has at least min(contentLength, sizeLimit, +// maxEagerPreGrow) bytes of capacity, so the bytes.Buffer.ReadFrom pumps +// inside parseUpload below skip the first round(s) of geometric grow on the +// common upload sizes — see #6541. The cap policy is the load-bearing piece +// here; it's extracted so unit tests can exercise the policy directly +// without spinning a real http upload. +func eagerPreGrow(bytesBuffer *bytes.Buffer, contentLength, sizeLimit int64) { + if contentLength <= 0 || contentLength > sizeLimit { + return + } + grow := contentLength + if grow > maxEagerPreGrow { + grow = maxEagerPreGrow + } + bytesBuffer.Grow(int(grow)) +} + type ParsedUpload struct { FileName string Data []byte @@ -35,6 +60,7 @@ type ParsedUpload struct { func ParseUpload(r *http.Request, sizeLimit int64, bytesBuffer *bytes.Buffer) (pu *ParsedUpload, e error) { bytesBuffer.Reset() + eagerPreGrow(bytesBuffer, r.ContentLength, sizeLimit) pu = &ParsedUpload{bytesBuffer: bytesBuffer} pu.PairMap = make(map[string]string) for k, v := range r.Header { diff --git a/weed/storage/needle/needle_parse_upload_alloc_test.go b/weed/storage/needle/needle_parse_upload_alloc_test.go new file mode 100644 index 000000000..3b65c289a --- /dev/null +++ b/weed/storage/needle/needle_parse_upload_alloc_test.go @@ -0,0 +1,104 @@ +package needle + +import ( + "bytes" + "testing" +) + +// TestEagerPreGrow is a regression guard for the volume-side amplifier in +// https://github.com/seaweedfs/seaweedfs/issues/6541. +// +// ParseUpload reads the multipart part body via bytes.Buffer.ReadFrom; if the +// receive buffer arrives with cap=0 (sync.Pool dropped the prior buffer), +// ReadFrom doubles capacity on each grow and pays 2-4x the chunk size in +// cumulative allocations. eagerPreGrow shaves the first few rounds off that +// grow chain by pre-sizing from r.ContentLength. +// +// Two policy invariants the cases below pin: +// +// 1. The cap. We never grow more than maxEagerPreGrow per request, so a +// misreported Content-Length or a slow/idle client cannot force +// per-request preallocation up to the configured sizeLimit. This is the +// review concern that landed during PR review and is what changed the +// test from a TotalAlloc bound to this structural form. +// 2. The validity gates. Negative / zero / oversized Content-Length and +// buffers that already have enough capacity should not trigger a Grow. +func TestEagerPreGrow(t *testing.T) { + const sizeLimit = int64(256 * 1024 * 1024) + + cases := []struct { + name string + startCap int + cl int64 + wantMinCap int + wantNoop bool // post: cap stays at startCap + }{ + { + name: "small content-length grows exactly", + cl: 1 * 1024 * 1024, + wantMinCap: 1 * 1024 * 1024, + }, + { + name: "content-length at cap grows to cap", + cl: maxEagerPreGrow, + wantMinCap: maxEagerPreGrow, + }, + { + name: "content-length above cap is clamped to cap", + cl: 200 * 1024 * 1024, + wantMinCap: maxEagerPreGrow, + }, + { + name: "content-length above sizeLimit is rejected", + cl: sizeLimit + 1, + wantNoop: true, + }, + { + name: "zero content-length is a no-op", + cl: 0, + wantNoop: true, + }, + { + name: "negative content-length (chunked encoding) is a no-op", + cl: -1, + wantNoop: true, + }, + { + name: "buffer with sufficient cap is left alone", + startCap: maxEagerPreGrow, + cl: 1 * 1024 * 1024, + wantMinCap: maxEagerPreGrow, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + bb := &bytes.Buffer{} + if c.startCap > 0 { + bb.Grow(c.startCap) + } + startCap := bb.Cap() + + eagerPreGrow(bb, c.cl, sizeLimit) + + if c.wantNoop { + if got := bb.Cap(); got != startCap { + t.Fatalf("eagerPreGrow(cl=%d, sizeLimit=%d) should not have grown: "+ + "cap=%d, want %d (regression: validity gate removed?)", + c.cl, sizeLimit, got, startCap) + } + return + } + if got := bb.Cap(); got < c.wantMinCap { + t.Fatalf("after eagerPreGrow(cl=%d): cap=%d, want >=%d", c.cl, got, c.wantMinCap) + } + // Hard upper bound: never above the cap (regardless of cl/sizeLimit). + // Note bytes.Buffer.Grow may round capacity up modestly, so allow + // a generous overshoot but still well below sizeLimit. + if got := int64(bb.Cap()); got > 2*int64(maxEagerPreGrow) { + t.Fatalf("after eagerPreGrow(cl=%d): cap=%d exceeds 2*maxEagerPreGrow=%d "+ + "(regression: cap removed?)", c.cl, got, 2*maxEagerPreGrow) + } + }) + } +}