Add BufWriter for Adapative Put / Multipart Upload#5431
Add BufWriter for Adapative Put / Multipart Upload#5431tustvold merged 2 commits intoapache:masterfrom
Conversation
object_store/src/buffered.rs
Outdated
| Self { | ||
| capacity, | ||
| store, | ||
| state: BufWriterState::Buffer(path, Vec::with_capacity(1024)), |
There was a problem hiding this comment.
It's by design to keep a 1024B buffer?
There was a problem hiding this comment.
This was a habit, as typically you want to avoid small bump allocations when pushing individual records. In this case poll_write is called with a slice of values, so this is not necessary. Will change
|
|
||
| impl BufWriter { | ||
| /// Create a new [`BufWriter`] from the provided [`ObjectStore`] and [`Path`] | ||
| pub fn new(store: Arc<dyn ObjectStore>, path: Path) -> Self { |
There was a problem hiding this comment.
An off-topic question: I remember we planned to support content_type and custom metadata. How can we accommodate this use case in our current API design?
| self.multipart_id = Some(id); | ||
| } | ||
| BufWriterState::Buffer(p, b) => { | ||
| let buf = std::mem::take(b); |
There was a problem hiding this comment.
In severe cases, poll_shutdown might return an error. Should we ensure that retrying poll_shutdown functions is safe?
There was a problem hiding this comment.
What do you mean by safe? What is the issue if poll_shutdown returns an error?
There was a problem hiding this comment.
Apologies for the confusion caused by my choice of the word "safe". I'm talking about this case:
- write some data in buffer.
- calling
poll_shutdown, return error like500 Internal Server. - calling
poll_shutdownagain for retrying.
But the buf has been take in the first call, so the final content could be wrong.
There was a problem hiding this comment.
Filed https://github.com/apache/arrow-rs/issues/5437 to track this, as I think there is a broader issue here
* Add BufWriter * Review feedback
3 participants
Which issue does this PR close?
Closes #5205
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?