hidrive: buffer small file updates from the global memory pool

Updating a file below --hidrive-upload-cutoff copied the whole file into
an append-grown slice so the request could be retried, costing roughly
twice the file size in transient allocations for every such update.

Buffer the file in a multipart.NewRW from rclone's global page pool
instead and return it to the pool once the request has finished. The
pool.RW is seekable so retries re-send the same buffer. Accounting is
applied as the buffer is sent so bandwidth limits and progress still
track the upload. The request now carries an explicit Content-Length
rather than being sent chunked.

The upload is bounded by the size the source declares - a source that
delivers more bytes than its declared size has the excess ignored, where
previously the stream was sent to EOF.

The FsPutRetry integration test covers the retry of a failed upload
request and checks the buffers are returned to the pool.

backend/hidrive/helpers.go

@@ -10,7 +10,6 @@ package hidrive
 // to be resolved prior to execution with resolvePath(...).
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"io"
@@ -27,7 +26,6 @@ import (
 	"github.com/rclone/rclone/lib/multipart"
 	"github.com/rclone/rclone/lib/pool"
 	"github.com/rclone/rclone/lib/ranges"
-	"github.com/rclone/rclone/lib/readers"
 	"github.com/rclone/rclone/lib/rest"
 	"golang.org/x/sync/errgroup"
 	"golang.org/x/sync/semaphore"
@@ -514,16 +512,16 @@ func (f *Fs) createFile(ctx context.Context, path string, content *pool.RW, modT
 }
 
 // overwriteFile updates the content of the file at the given path
-// with the content of the io.ReadSeeker.
+// with the content of the buffer.
 // If the file does not exist it will be created.
 // The maximum size of the content is limited by MaximumUploadBytes.
-// The io.ReadSeeker should be resettable by seeking to its start.
+// The caller remains responsible for closing the buffer.
 // If modTime is not the zero time instant,
 // it will be set as the file's modification time after the operation.
 //
 // This returns fs.ErrorDirNotFound
 // if the parent directory of the file is not found.
-func (f *Fs) overwriteFile(ctx context.Context, path string, content io.ReadSeeker, modTime time.Time) (*api.HiDriveObject, error) {
+func (f *Fs) overwriteFile(ctx context.Context, path string, content *pool.RW, modTime time.Time) (*api.HiDriveObject, error) {
 	parameters := api.NewQueryParameters()
 	parameters.SetFileInDirectory(path)
 
@@ -535,12 +533,14 @@ func (f *Fs) overwriteFile(ctx context.Context, path string, content io.ReadSeek
 		}
 	}
 
+	contentLength := content.Size()
 	opts := rest.Opts{
-		Method:      "PUT",
-		Path:        "/file",
-		Body:        content,
-		ContentType: "application/octet-stream",
-		Parameters:  parameters.Values,
+		Method:        "PUT",
+		Path:          "/file",
+		Body:          content,
+		ContentType:   "application/octet-stream",
+		ContentLength: &contentLength,
+		Parameters:    parameters.Values,
 	}
 
 	var result api.HiDriveObject
@@ -864,29 +864,6 @@ func unwrapAccounting(reader io.Reader) (unwrapped io.Reader, acc *accounting.Ac
 	return unwrapped, acc
 }
 
-// accountedReadSeeker reads through any accounting wrapped around a
-// buffered reader while seeking the buffer underneath it,
-// so a retry can rewind the buffer without copying it.
-type accountedReadSeeker struct {
-	io.Reader
-	io.Seeker
-}
-
-// cachedReader returns a version of the reader that caches its contents and
-// can therefore be reset using Seek.
-//
-// Readers which are already seekable buffers are used as they are,
-// even when wrapped in accounting.
-func cachedReader(reader io.Reader) io.ReadSeeker {
-	unwrapped, _ := accounting.UnWrap(reader)
-	switch unwrapped.(type) {
-	case *bytes.Reader, *readers.RepeatableReader:
-		return accountedReadSeeker{Reader: reader, Seeker: unwrapped.(io.Seeker)}
-	}
-
-	return readers.NewRepeatableReader(reader)
-}
-
 // readerForChunk reads up to length bytes from reader into a buffer
 // from the global memory pool and returns it
 // along with the number of bytes that have been read from reader.

backend/hidrive/hidrive.go

@@ -30,6 +30,7 @@ import (
 	"github.com/rclone/rclone/fs/hash"
 	"github.com/rclone/rclone/lib/oauthutil"
 	"github.com/rclone/rclone/lib/pacer"
+	"github.com/rclone/rclone/lib/pool"
 	"github.com/rclone/rclone/lib/rest"
 )
 
@@ -978,7 +979,12 @@ func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, op
 		// Metadata should be updated even if the upload fails.
 		info, metaErr = o.fs.fetchMetadataForPath(ctx, resolvedPath, api.HiDriveObjectWithMetadataFields)
 	} else {
-		info, err = o.fs.overwriteFile(ctx, resolvedPath, cachedReader(in), modTime)
+		var content *pool.RW
+		content, _, err = readerForChunk(in, src.Size())
+		if err == nil {
+			info, err = o.fs.overwriteFile(ctx, resolvedPath, content, modTime)
+			_ = content.Close()
+		}
 		metaErr = err
 	}