Skip to content

Commit

Permalink
fix: Increment S3 Request Count on Success (#1026)
Browse files Browse the repository at this point in the history
We were previously incrementing S3 request count in all cases. However,
this was misleading as the metrics indicated a significantly higher
amount of requests than expected, and the metric was being used as a
proxy for cost of the service. Research indicated that many requests
were in fact failing due to a dispatch error, shown below.

`GetObjectBytesError(DispatchFailure(DispatchFailure { source:
ConnectorError { kind: Io, source: hyper::Error(Connect,
ConnectError("dns error", Custom { kind: Uncategorized, error: "failed
to lookup address information: nodename nor servname provided, or not
known" })), connection: Unknown } }))`

In order to more accurately gauge actual S3 usage by the service, I've
changed the increment counter to occur below the S3 get request, so that
only successful requests get counted. However, it was observed that this
metric is somehow still inaccurate, reporting an inconsistent number
compared to the actual requests made, which were verified through cache
size and logs. Regardless, this should still be an improvement to the
metric.
  • Loading branch information
darunrs authored Nov 12, 2024
1 parent 8cbd394 commit 7679e0d
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions block-streamer/src/lake_s3_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,12 @@ impl LakeS3Client {
let prefix = prefix.to_owned();

async move {
metrics::LAKE_S3_GET_REQUEST_COUNT.inc();

let object = s3_client.get_object(&bucket, &prefix).await?;

let bytes = object.body.collect().await?.into_bytes().to_vec();

metrics::LAKE_S3_GET_REQUEST_COUNT.inc();

Ok(bytes)
}
.boxed()
Expand Down

0 comments on commit 7679e0d

Please sign in to comment.