Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update S3 flush to not have last part greater than defined chunkSize #4327

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arvind-choudhary-h
Copy link

Fixes: #3873

The new implementation ensures no part size exceeds the defined Chunk size. In the screenshot shared, this is before the changes where the size of the last part was 6579438 which is more than the defined 5242880

The following logic is removed which was responsible for adding the last part bytes into the buf

-	if w.pending.Len() > 0 && w.pending.Len() < int(w.driver.ChunkSize) {
-		if _, err := buf.Write(w.pending.data); err != nil {
-			return err
-		}
-		w.pending.Clear()
-	}

Screenshot 2024-04-16 at 1 40 39 PM

Signed-off-by: Arvind Choudhary <arvind.choudhary@harness.io>
@milosgajdos
Copy link
Member

Please, read the whole discussion here: #3940

That's a PR that also attempted adding a recursive flush call, just like this one.

@arvind-choudhary-h
Copy link
Author

Thanks for the review!
I went through #3940 earlier and this change was not working for me in the latest main branch; probably due to the deprecated MultipartCombineSmallPart value. Let me revisit the design and get back to this.

@arvind-choudhary-h arvind-choudhary-h marked this pull request as draft April 17, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multipart upload issues with Cloudflare R2
2 participants