From bd217a1082809a8644cd7c44a05eb3e89b6e94af Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 31 Aug 2026 12:37:44 +0100 Subject: [PATCH] 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 | 43 +++++++++----------------------------- backend/hidrive/hidrive.go | 8 ++++++- 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/backend/hidrive/helpers.go b/backend/hidrive/helpers.go index 3e7ac374d..b0525d647 100644 --- a/backend/hidrive/helpers.go +++ b/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. diff --git a/backend/hidrive/hidrive.go b/backend/hidrive/hidrive.go index 90f2652dd..8b4e1d9a9 100644 --- a/backend/hidrive/hidrive.go +++ b/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 }