From 337f762c799998331308a047a05d143e0704c4ea Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Sun, 30 Aug 2026 17:35:18 +0100
Subject: [PATCH] shade: stop copying each upload chunk into a second heap
 buffer

WriteChunk copied the whole chunk, which lib/multipart already hands over
in a buffer from the global memory pool, into a bytes.Buffer so that
retries could re-send it. That doubled the per-part memory and made a
fresh chunk-sized heap allocation (64 MiB by default) for every part,
times the upload concurrency.

The chunk reader is seekable, so find its size with Seek and rewind it
inside the pacer closure instead, sending the pooled buffer directly.

Also fix the error for a part that fails to upload, which formatted the
buffer instead of the part number.
---
 backend/shade/upload.go               | 23 +++++-----
 backend/shade/upload_internal_test.go | 65 +++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 11 deletions(-)
 create mode 100644 backend/shade/upload_internal_test.go

diff --git a/backend/shade/upload.go b/backend/shade/upload.go
index e86cecc02..66660e836 100644
--- a/backend/shade/upload.go
+++ b/backend/shade/upload.go
@@ -3,7 +3,6 @@
 package shade
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"io"
@@ -155,17 +154,15 @@ func (s *shadeChunkWriter) WriteChunk(ctx context.Context, chunkNumber int, read
 		return 0, err
 	}
 
-	// Read chunk
-	var chunk bytes.Buffer
-	n, err := io.Copy(&chunk, reader)
+	// Find the chunk size
+	n, err := reader.Seek(0, io.SeekEnd)
+	if err != nil {
+		return 0, fmt.Errorf("failed to read chunk: %w", err)
+	}
 
 	if n == 0 {
 		return 0, nil
 	}
-
-	if err != nil {
-		return 0, fmt.Errorf("failed to read chunk: %w", err)
-	}
 	// Get presigned URL for this part
 	var partURL api.PartURL
 
@@ -201,14 +198,18 @@ func (s *shadeChunkWriter) WriteChunk(ctx context.Context, chunkNumber int, read
 	}
 
 	err = s.f.pacer.Call(func() (bool, error) {
-		// Use a fresh reader for each attempt so retries resend the whole chunk
-		opts.Body = bytes.NewReader(chunk.Bytes())
+		// Rewind the chunk so each attempt sends it in full
+		_, err = reader.Seek(0, io.SeekStart)
+		if err != nil {
+			return false, err
+		}
+		opts.Body = reader
 		uploadRes, err = s.f.srv.Call(ctx, &opts)
 		return shouldRetry(ctx, uploadRes, err)
 	})
 
 	if err != nil {
-		return 0, fmt.Errorf("failed to upload part %d: %w", chunk, err)
+		return 0, fmt.Errorf("failed to upload part %d: %w", chunkNumber+1, err)
 	}
 
 	if uploadRes.StatusCode != http.StatusOK && uploadRes.StatusCode != http.StatusCreated {
diff --git a/backend/shade/upload_internal_test.go b/backend/shade/upload_internal_test.go
new file mode 100644
index 000000000..899e8ac9c
--- /dev/null
+++ b/backend/shade/upload_internal_test.go
@@ -0,0 +1,65 @@
+package shade
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/lib/pacer"
+	"github.com/rclone/rclone/lib/rest"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// newTestChunkWriter returns a chunk writer pointed at server, which must
+// answer the part URL request with a URL back to itself and the part PUT.
+func newTestChunkWriter(t *testing.T, server *httptest.Server) *shadeChunkWriter {
+	ctx := context.Background()
+	f := &Fs{
+		srv:      rest.NewClient(server.Client()),
+		endpoint: server.URL,
+		drive:    "drive",
+		pacer:    fs.NewPacer(ctx, pacer.NewDefault(pacer.MinSleep(time.Millisecond), pacer.MaxSleep(time.Millisecond))),
+		token:    "token",
+		tokenExp: time.Now().Add(time.Hour),
+	}
+	return &shadeChunkWriter{initToken: "init", f: f}
+}
+
+// partServer returns a test server which hands out a part URL pointing at
+// itself and passes part PUTs to put.
+func partServer(t *testing.T, put func(w http.ResponseWriter, body []byte)) *httptest.Server {
+	var server *httptest.Server
+	server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if strings.Contains(r.URL.Path, "/upload/multipart/part/") {
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = fmt.Fprintf(w, `{"url":%q}`, server.URL+"/put")
+			return
+		}
+		body, err := io.ReadAll(r.Body)
+		require.NoError(t, err)
+		put(w, body)
+	}))
+	return server
+}
+
+// TestWriteChunkError checks that a part which keeps failing reports the
+// part number in the error.
+func TestWriteChunkError(t *testing.T) {
+	server := partServer(t, func(w http.ResponseWriter, body []byte) {
+		w.WriteHeader(http.StatusInternalServerError)
+	})
+	defer server.Close()
+
+	s := newTestChunkWriter(t, server)
+	_, err := s.WriteChunk(context.Background(), 2, bytes.NewReader([]byte("shade")))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to upload part 3:")
+}