fix(volume): pre-size ParseUpload buffer to request ContentLength (#9421)

* fix(volume): pre-size ParseUpload buffer to request ContentLength The volume server's PostHandler reads the multipart upload body via bytes.Buffer.ReadFrom inside parseUpload. The buffer comes from a sync.Pool and may have cap=0 when the pool dropped the prior entry, which makes ReadFrom geometric-grow on each chunk: a 64 MiB upload allocates roughly 1+2+4+...+64 ≈ 128 MiB just to receive the body. Under concurrent uploads (every s3 chunk-copy lands here on the destination volume) this is one of the main contributors to the runaway-RSS pattern in #6541 — pprof shows ~458 MiB cum in parseUpload's bytes.Buffer.ReadFrom under Harbor-style assemble load. Grow the buffer once up front, bounded by the existing sizeLimit so a misreported Content-Length can't over-allocate. The receive then fills in place. Add a regression test that drives ParseUpload with a 16 MiB multipart body and bounds TotalAlloc at 1.5x the chunk size (pre-fix measures ~4x, so the bound trips deterministically). * fix(volume): guard ParseUpload pre-grow against int overflow on 32-bit Address PR review feedback: r.ContentLength is int64, and on 32-bit platforms int is 32 bits wide, so int(r.ContentLength) for a value above math.MaxInt32 wraps negative and bytes.Buffer.Grow panics with "bytes.Buffer.Grow: negative count". Skip the pre-grow optimization in that range; the existing geometric-grow path remains correct, just slightly more allocator pressure for that one call. 64-bit platforms (math.MaxInt == math.MaxInt64) are unaffected — the guard only kicks in for 32-bit builds with very large sizeLimit. * fix(volume): cap ParseUpload pre-grow at 4 MiB to bound DoS surface Address PR review: pre-growing the receive buffer to r.ContentLength trusts the header before any body bytes arrive. A bad header or slow / idle client could declare a large Content-Length up to sizeLimit (256 MiB by default for volume writes) and force per-request preallocation without sending data, turning many concurrent slow connections into avoidable memory pressure. Cap eager pre-grow at maxEagerPreGrow (4 MiB). Larger uploads still benefit from the higher starting cap and fall back to ReadFrom's grow path for the remainder. Per-request waste from a misreported Content-Length is now bounded at 4 MiB regardless of sizeLimit. Extract the policy as eagerPreGrow so the unit test can exercise the gates structurally — replaces the prior TotalAlloc bound (which became uninformative once savings were capped at 4 MiB).
2026-08-18 21:26:56 +00:00 · 2026-05-10 12:08:24 -07:00
parent 926a8e9351
commit c917110124
2 changed files with 130 additions and 0 deletions
@@ -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 {
@@ -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)
+			}
+		})
+	}
+}