YuangGao commented on code in PR #3179:
URL: https://github.com/apache/iceberg-rust/pull/3179#discussion_r4044253708
##########
crates/storage/opendal/src/s3.rs:
##########
@@ -36,6 +36,36 @@ use url::Url;
use crate::utils::{from_opendal_error, is_truthy};
+/// S3 rejects a non-final part smaller than this.
+const MULTIPART_PART_SIZE_MIN: u64 = 5 * 1024 * 1024;
+
+/// Matches Java `S3FileIOProperties.MULTIPART_SIZE_DEFAULT`.
+pub(crate) fn default_multipart_part_size() -> u64 {
+ 32 * 1024 * 1024
+}
+
+/// Parse iceberg props to s3 multipart upload part size.
+pub(crate) fn s3_multipart_part_size_parse(m: &HashMap<String, String>) ->
Result<u64> {
+ let Some(value) = m.get(S3_MULTIPART_PART_SIZE_BYTES) else {
+ return Ok(default_multipart_part_size());
+ };
+ let part_size = value.parse::<u64>().map_err(|e| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ format!("Invalid {S3_MULTIPART_PART_SIZE_BYTES}: {value}: {e}"),
+ )
+ })?;
+ if part_size < MULTIPART_PART_SIZE_MIN {
Review Comment:
Added `MULTIPART_PART_SIZE_MAX` and the mirror check. Kept
unwrap_or(usize::MAX) rather than expect though — on a 32-bit target usize::MAX
is under 4 GiB, so a validated 5 GiB value still fails the cast and the expect
would be a panic reachable from config.
One correction: OpenDAL already clamps chunk to the service's
`write_multi_max_size` (5 GiB for S3), so an oversized value was silently
reduced rather than failing mid-upload
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
