Xuanwo commented on code in PR #2143:
URL:
https://github.com/apache/incubator-opendal/pull/2143#discussion_r1186711358
##########
core/src/services/gcs/writer.rs:
##########
@@ -27,6 +27,8 @@ use crate::ops::OpWrite;
use crate::raw::*;
use crate::*;
+/// It's recommended that you use at least 8 MiB for the chunk size.
+const DEFAULT_GCS_MINIMUM_PART_SIZE: usize = 8 * 1024 * 1024;
Review Comment:
GCS requires size aligned with 256KiB, so `MINIMUM` is incorrect. And gcs
doesn't have concept of `PART_SIZE`.
How about use `DEFAULT_WRITE_FIXED_SIZE` instead?
##########
core/src/services/oss/writer.rs:
##########
@@ -28,6 +28,7 @@ use crate::ops::OpWrite;
use crate::raw::*;
use crate::*;
+const DEFAULT_OSS_MINIMUM_PART_SIZE: usize = 8 * 1024 * 1024;
Review Comment:
How about using `DEFAULT_WRITE_MIN_SIZE`?
##########
core/src/services/gcs/backend.rs:
##########
@@ -236,6 +239,23 @@ impl GcsBuilder {
};
self
}
+
+ /// 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) ->
Result<&mut Self> {
+ if fixed_buffer_size.checked_rem_euclid(256 * 1024).is_none() {
+ self.write_fixed_size = Some(fixed_buffer_size);
+ } else {
+ return Err(
+ Error::new(ErrorKind::ConfigInvalid, "The buffer size is
misconfigured")
Review Comment:
Oh, sorry for my misleading review. I expect to check this value inside
`build` instead in `write_fixed_size` itself.
--
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]
