Xuanwo commented on code in PR #2143:
URL:
https://github.com/apache/incubator-opendal/pull/2143#discussion_r1185790846
##########
core/src/services/oss/writer.rs:
##########
@@ -41,7 +41,8 @@ pub struct OssWriter {
}
impl OssWriter {
- pub fn new(core: Arc<OssCore>, path: &str, op: OpWrite, buffer_size:
Option<usize>) -> Self {
+ pub fn new(core: Arc<OssCore>, path: &str, op: OpWrite) -> Self {
+ let buffer_size = core.writer_buffer_size.unwrap_or(8 * 1024 * 1024);
Review Comment:
If we have a default value for `write_min_size`, we should store a `usize`
directly.
##########
core/src/services/oss/backend.rs:
##########
@@ -294,10 +295,15 @@ impl OssBuilder {
self
}
- /// set the buffer size of unsized write.
- pub fn write_buffer_size(&mut self, buffer_size: &str) -> &mut Self {
- let buffer_size = buffer_size.parse::<usize>().unwrap();
- self.write_buffer_size = Some(buffer_size);
+ /// set the minimum value of unsized write, valid values: 100 KB to 5 GB
+ /// Reference: [OSS Multipart
upload](https://www.alibabacloud.com/help/en/object-storage-service/latest/multipart-upload-6)
+ pub fn write_buffer_size(&mut self, write_buffer_size: &str) -> &mut Self {
Review Comment:
- write_buffer_size -> write_min_size
- `&str` -> `usize`
##########
core/src/services/oss/backend.rs:
##########
@@ -294,10 +295,15 @@ impl OssBuilder {
self
}
- /// set the buffer size of unsized write.
- pub fn write_buffer_size(&mut self, buffer_size: &str) -> &mut Self {
- let buffer_size = buffer_size.parse::<usize>().unwrap();
- self.write_buffer_size = Some(buffer_size);
+ /// set the minimum value of unsized write, valid values: 100 KB to 5 GB
+ /// Reference: [OSS Multipart
upload](https://www.alibabacloud.com/help/en/object-storage-service/latest/multipart-upload-6)
+ pub fn write_buffer_size(&mut self, write_buffer_size: &str) -> &mut Self {
+ let write_buffer_size = write_buffer_size.parse::<usize>().unwrap();
Review Comment:
The same, check and return error in `build`.
##########
core/src/services/gcs/backend.rs:
##########
@@ -238,10 +240,17 @@ impl GcsBuilder {
self
}
- /// set the buffer size of unsized write.
- pub fn write_buffer_size(&mut self, buffer_size: &str) -> &mut Self {
- let buffer_size = buffer_size.parse::<usize>().unwrap();
- self.write_buffer_size = Some(buffer_size);
+ /// The buffer size should be a multiple of 256 KiB (256 x 1024 bytes),
unless it's the last chunk that completes the upload.
+ /// Larger chunk sizes typically make uploads faster, but note that
there's a tradeoff between speed and memory usage.
+ /// It's recommended that you use at least 8 MiB for the chunk size.
+ /// Reference: [Perform resumable
uploads](https://cloud.google.com/storage/docs/performing-resumable-uploads)
+ pub fn write_fixed_size(&mut self, fixed_buffer_size: usize) -> &mut Self {
+ if fixed_buffer_size.checked_rem_euclid(256 * 1024).is_none() {
Review Comment:
We should check this and return an error during `build`
##########
core/src/services/oss/backend.rs:
##########
@@ -294,10 +295,15 @@ impl OssBuilder {
self
}
- /// set the buffer size of unsized write.
- pub fn write_buffer_size(&mut self, buffer_size: &str) -> &mut Self {
- let buffer_size = buffer_size.parse::<usize>().unwrap();
- self.write_buffer_size = Some(buffer_size);
+ /// set the minimum value of unsized write, valid values: 100 KB to 5 GB
Review Comment:
The part should be larger than 5MiB
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
