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

Using WriteMultipart::put results in 0 bytes being written #5743

Closed
orf opened this issue May 10, 2024 · 2 comments · Fixed by #5744 or #5746
Closed

Using WriteMultipart::put results in 0 bytes being written #5743

orf opened this issue May 10, 2024 · 2 comments · Fixed by #5744 or #5746
Labels
bug object-store Object Store Interface

Comments

@orf
Copy link
Contributor

orf commented May 10, 2024

WriteMultipart::put seems to be trivially broken. The method depends on the size of the buffer (via the content_length() method):

pub fn put(&mut self, mut bytes: Bytes) {
while !bytes.is_empty() {
let remaining = self.chunk_size - self.buffer.content_length();
if bytes.len() < remaining {
self.buffer.push(bytes);
return;
}
self.buffer.push(bytes.split_to(remaining));
let buffer = std::mem::take(&mut self.buffer);
self.put_part(buffer.into())
}
}

However this is not updated by push():

pub fn push(&mut self, bytes: Bytes) {
if !self.in_progress.is_empty() {
let completed = std::mem::take(&mut self.in_progress);
self.completed.push(completed.into())
}
self.completed.push(bytes)
}

Repeated calls to .put() will not modify buffer.len (it will always be 0), so no part will ever be uploaded to S3.

Reproduction: foobar will have 0 bytes, even if you write more than 25mb of data.

let store = AmazonS3Builder::new().with_bucket_name("foo").build()?;
let upload = store.put_multipart("foobar").await?;
let uploader = WriteMultipart::new_with_chunk_size(upload, 1024 * 1024 * 25);
uploader.put(Bytes::from_static(b"yo"));
uploader.put(Bytes::from_static(b"yo2"));
uploader.put(Bytes::from_static(b"yo3"));
uploader.finish().await.unwrap();
@tustvold
Copy link
Contributor

label_issue.py automatically added labels {'object-store'} from #5744

@orf
Copy link
Contributor Author

orf commented May 10, 2024

Thanks for the super quick fix @tustvold <3

tustvold added a commit to tustvold/arrow-rs that referenced this issue May 10, 2024
tustvold added a commit that referenced this issue May 11, 2024
* Add additional WriteMultipart tests (#5743)

* Clippy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug object-store Object Store Interface
Projects
None yet
2 participants