Bump forked google-api-go-client to v0.277.0-patched#4826
Conversation
v0.277.0-patched is the proper v0.277.0 google-api-go-client tag with c427c0d9 from our fork allowing to pass pre-allocated buffer on file upload. The diff is huge due to the changes between v0.34 and v0.277. To validate the actual changes, one can take a look at: - how our changes were applied before: scylladb/google-api-go-client@c427c0d - how our changes were applied after rebase: scylladb/google-api-go-client@292c1f5 This commit also removes the specific version from the replace directive, as it doesn't make sense in our case, since we don't expect newer versions of google-api-go-client to include this change. Fixes https://scylladb.atlassian.net/browse/CLOUD-1909
fa667ff to
718b9ef
Compare
|
Can you validate with i.e. opus that the WithBuffer is included into the upload operations and Go doesn't allocate memory for every consecutive transfer but reuses the buffer ? |
|
@copilot validate that the changes from commit c427c0d9 were correctly applied on top of v0.277.0 of google-api-go-client and they still achieve the same goal. Focus on ensuring that the WithBuffer is included into the upload operations and Go doesn't allocate memory for every consecutive transfer but reuses the buffer. |
I've validated that the
The implementation correctly achieves the same goal as the original fork: allowing a pre-allocated buffer to be passed into upload operations so that memory is reused across transfers instead of being allocated fresh for every upload. |
|
@karol-kokoszka see #4826 (comment) - do you want me to run some SCT against GCP and take a look at the golang memory metrics during backup execution? |
It's ok, feel free to merge. |
v0.277.0-patched is the proper v0.277.0 google-api-go-client tag with c427c0d9 from our fork allowing to pass pre-allocated buffer on file upload. The diff is huge due to the changes between v0.34 and v0.277.
To validate the actual changes, one can take a look at:
This commit also removes the specific version from the replace directive, as it doesn't make sense in our case, since we don't expect newer versions of google-api-go-client to include this change.
git range-diff
1: c427c0d93a ! 1: 292c1f5934 smpatch: gensupport: Allow user to provide his own buffer used for uploading @@ ## Metadata ## -Author: Maciej Zimnoch <maciej@scylladb.com> +Author: Michał Leszczyński <2000michal@wp.pl> ## Commit message ## - smpatch: gensupport: Allow user to provide his own buffer used for uploading + chore(gensupport): cherry-pick c427c0d9 + + c427c0d9 is the reason for our fork existence, as it makes it + possible to reuse reallocated buffers for uploading files. + This commit cherry-picks it on top of v0.277.0. ## googleapi/googleapi.go ## @@ googleapi/googleapi.go: type MediaOption interface { @@ googleapi/googleapi.go: type MediaOption interface { func (ct contentTypeOption) setOptions(o *MediaOptions) { @@ googleapi/googleapi.go: type MediaOptions struct { - ForceEmptyContentType bool - - ChunkSize int + ChunkRetryDeadline time.Duration + ChunkTransferTimeout time.Duration + EnableAutoChecksum bool + + Buffer []byte } @@ internal/gensupport/buffer.go: func NewMediaBuffer(media io.Reader, chunkSize in return &MediaBuffer{media: media, chunk: make([]byte, 0, chunkSize)} } -+// NewMediaBuffer initializes a MediaBuffer. ++// NewMediaBufferWithBuffer initializes a MediaBuffer. +func NewMediaBufferWithBuffer(media io.Reader, chunkSize int, buffer []byte) *MediaBuffer { + // If buffer isn't long enough, allocate new one. + if cap(buffer) < chunkSize { @@ internal/gensupport/media.go: func typeHeader(contentType string) textproto.MIME // // After PrepareUpload has been called, media should no longer be used: the // media content should be accessed via one of the return values. --func PrepareUpload(media io.Reader, chunkSize int) (r io.Reader, mb *MediaBuffer, singleChunk bool) { -+func PrepareUpload(media io.Reader, chunkSize int, buffer []byte) (r io.Reader, mb *MediaBuffer, singleChunk bool) { +-func PrepareUpload(media io.Reader, chunkSize int, enableAutoChecksum bool) (r io.Reader, mb *MediaBuffer, singleChunk bool) { ++func PrepareUpload(media io.Reader, chunkSize int, enableAutoChecksum bool, buffer []byte) (r io.Reader, mb *MediaBuffer, singleChunk bool) { if chunkSize == 0 { // do not chunk return media, nil, true } @@ internal/gensupport/media.go: func typeHeader(contentType string) textproto.MIME + } else { + mb = NewMediaBuffer(media, chunkSize) + } -+ + mb.enableAutoChecksum = enableAutoChecksum _, _, _, err := mb.Chunk() // If err is io.EOF, we can upload this in a single request. Otherwise, err is - // either nil or a non-EOF error. If it is the latter, then the next call to @@ internal/gensupport/media.go: func NewInfoFromMedia(r io.Reader, options []googleapi.MediaOption) *MediaInfo { - if !opts.ForceEmptyContentType { - r, mi.mType = DetermineContentType(r, opts.ContentType) } -- mi.media, mi.buffer, mi.singleChunk = PrepareUpload(r, opts.ChunkSize) -+ mi.media, mi.buffer, mi.singleChunk = PrepareUpload(r, opts.ChunkSize, opts.Buffer) + mi.chunkRetryDeadline = opts.ChunkRetryDeadline + mi.chunkTransferTimeout = opts.ChunkTransferTimeout +- mi.media, mi.buffer, mi.singleChunk = PrepareUpload(r, opts.ChunkSize, opts.EnableAutoChecksum) ++ mi.media, mi.buffer, mi.singleChunk = PrepareUpload(r, opts.ChunkSize, opts.EnableAutoChecksum, opts.Buffer) return mi } ## internal/gensupport/media_test.go ## -@@ internal/gensupport/media_test.go: func TestDetermineContentType(t *testing.T) { +@@ internal/gensupport/media_test.go: import ( + "io" + mathrand "math/rand" + "net/http" ++ "reflect" + "strings" + "testing" + "time" +@@ internal/gensupport/media_test.go: import ( func TestNewInfoFromMedia(t *testing.T) { const textType = "text/plain; charset=utf-8" -+ chunkBuffer := make([]byte, 0) ++ chunkBuffer := make([]byte, 0, googleapi.DefaultUploadChunkSize) for _, test := range []struct { - desc string - r io.Reader - opts []googleapi.MediaOption - wantType string - wantMedia, wantBuffer, wantSingleChunk bool -+ desc string -+ r io.Reader -+ opts []googleapi.MediaOption -+ wantType string -+ wantChunkBuffer []byte -+ wantMedia, wantAnyBuffer, wantSingleChunk bool +- wantDeadline time.Duration ++ desc string ++ r io.Reader ++ opts []googleapi.MediaOption ++ wantType string ++ wantMedia, wantAnyBuffer, wantBuffer, wantSingleChunk bool ++ wantChunkBuffer []byte ++ wantDeadline time.Duration }{ { desc: "an empty reader results in a MediaBuffer with a single, empty chunk", @@ internal/gensupport/media_test.go: func TestNewInfoFromMedia(t *testing.T) { wantType: "application/octet-stream", - wantBuffer: true, + wantAnyBuffer: true, -+ wantSingleChunk: false, -+ }, + wantSingleChunk: false, + }, + { + desc: "WithBuffer is observed", + r: new(bytes.Buffer), + opts: []googleapi.MediaOption{googleapi.WithBuffer(chunkBuffer)}, + wantType: textType, + wantChunkBuffer: chunkBuffer, - wantSingleChunk: false, - }, ++ wantSingleChunk: true, ++ }, } { + + mi := NewInfoFromMedia(test.r, test.opts) @@ internal/gensupport/media_test.go: func TestNewInfoFromMedia(t *testing.T) { if got, want := (mi.media != nil), test.wantMedia; got != want { t.Errorf("%s: media non-nil: got %t, want %t", test.desc, got, want) @@ internal/gensupport/media_test.go: func TestNewInfoFromMedia(t *testing.T) { t.Errorf("%s: singleChunk: got %t, want %t", test.desc, got, want) ## internal/gensupport/resumable_test.go ## -@@ internal/gensupport/resumable_test.go: func TestRetry_EachChunkHasItsOwnRetryDeadline(t *testing.T) { +@@ internal/gensupport/resumable_test.go: func TestUploadChecksum(t *testing.T) { } } } @@ internal/gensupport/resumable_test.go: func TestRetry_EachChunkHasItsOwnRetryDea + tr := &interruptibleTransport{ + buf: make([]byte, 0, mediaSize), + events: []event{ -+ {"bytes 0-255/*", 200}, ++ {"bytes 0-255/*", 200, 0}, + }, + bodies: bodyTracker{}, + }Fixes https://scylladb.atlassian.net/browse/CLOUD-1909