Skip to content

Commit

Permalink
Fix an issue where mountpoint-s3-client could interpret a HTTP 206 Pa…
Browse files Browse the repository at this point in the history
…rtial success response as an error (#917)

We are removing a workaround in mountpoint-s3-client that reduced the number of requests to S3 and is no longer needed. When introduced in #285, the workaround used a default CRT meta-request instead of an auto-ranged-get for small requests, which avoided a redundant HeadObject request that the CRT performed on every auto-ranged-get. Since then, the CRT has been updated to avoid the extra requests when a range is specified, so we can always use auto-ranged-get.

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
Co-authored-by: Alessandro Passaro <alexpax@amazon.com>
  • Loading branch information
passaro and Alessandro Passaro authored Jun 17, 2024
1 parent 78df1ae commit e299e2b
Showing 1 changed file with 4 additions and 18 deletions.
22 changes: 4 additions & 18 deletions mountpoint-s3-client/src/s3_crt_client/get_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,27 +46,13 @@ impl S3CrtClient {
.map_err(S3RequestError::construction_failure)?;
}

// Only use the CRT auto-ranged-get machinery for requests larger than the part size, or
// unknown lengths. This avoids the machinery's HeadObject requests for small/random
// requests. For auto-ranged-gets, the CRT takes care of adjusting the offset returned to
// the body callback to include the range start, but for manual requests we need to do it
// ourselves with `range_start`.
let (request_type, range_start) = if let Some(range) = range {
if let Some(range) = range {
// Range HTTP header is bounded below *inclusive*
let range_value = format!("bytes={}-{}", range.start, range.end.saturating_sub(1));
message
.set_header(&Header::new("Range", range_value))
.map_err(S3RequestError::construction_failure)?;

let length = range.end.saturating_sub(range.start);
if length >= self.inner.part_size as u64 {
(MetaRequestType::GetObject, 0)
} else {
(MetaRequestType::Default, range.start)
}
} else {
(MetaRequestType::GetObject, 0)
};
}

let key = format!("/{key}");
message
Expand All @@ -77,11 +63,11 @@ impl S3CrtClient {

let request = self.inner.make_meta_request(
message,
request_type,
MetaRequestType::GetObject,
span,
|_, _| (),
move |offset, data| {
let _ = sender.unbounded_send(Ok((range_start + offset, data.into())));
let _ = sender.unbounded_send(Ok((offset, data.into())));
},
move |result| {
if result.is_err() {
Expand Down

0 comments on commit e299e2b

Please sign in to comment.