Skip to content

Commit 8560af9

Browse files
committed
chore(pegboard): send artifact image size from workflow instead of fetching with HEAD
1 parent a220919 commit 8560af9

File tree

1 file changed

+22
-69
lines changed

1 file changed

+22
-69
lines changed

packages/edge/infra/client/manager/src/image_download_handler.rs

Lines changed: 22 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -99,28 +99,28 @@ impl ImageDownloadHandler {
9999
let mut conn = ctx.sql().await?;
100100
let mut tx = conn.begin().await?;
101101

102-
let ((cache_count, images_dir_size), image_download_size) = tokio::try_join!(
103-
async {
104-
// Get total size of images directory. Note that it doesn't matter if this doesn't
105-
// match the actual fs size because it should either be exactly at or below actual fs
106-
// size. Also calculating fs size manually is expensive.
107-
sqlx::query_as::<_, (i64, i64)>(indoc!(
108-
"
109-
SELECT COUNT(size), COALESCE(SUM(size), 0) FROM images_cache
110-
",
111-
))
112-
.fetch_one(&mut *tx)
113-
.await
114-
.map_err(Into::<anyhow::Error>::into)
115-
},
116-
// NOTE: The image size here is somewhat misleading because its only the size of the
117-
// downloaded archive and not the total disk usage after it is unpacked. However, this is
118-
// good enough
119-
self.fetch_image_download_size(ctx, image_config),
120-
)?;
102+
// Get total size of images directory. Note that it doesn't matter if this doesn't
103+
// match the actual fs size because it should either be exactly at or below actual fs
104+
// size. Also calculating fs size manually is expensive.
105+
let (cache_count, images_dir_size) = sqlx::query_as::<_, (i64, i64)>(indoc!(
106+
"
107+
SELECT COUNT(size), COALESCE(SUM(size), 0) FROM images_cache
108+
",
109+
))
110+
.fetch_one(&mut *tx)
111+
.await
112+
.map_err(Into::<anyhow::Error>::into)?;
121113

122114
// Prune images
123-
let (removed_count, removed_bytes) = if images_dir_size as u64 + image_download_size
115+
//
116+
// HACK: The artifact_size_bytes here is somewhat misleading because its only the size of the
117+
// downloaded archive and not the total disk usage after it is unpacked. However, this is size
118+
// is recalculated later once decompressed, so this will only ever exceed the cache
119+
// size limit in edge cases by `actual size - compressed size`. In this situation,
120+
// that extra difference is already reserved on the file system by the actor
121+
// itself.
122+
let (removed_count, removed_bytes) = if images_dir_size as u64
123+
+ image_config.artifact_size_bytes
124124
> ctx.config().images.max_cache_size()
125125
{
126126
// Fetch as many images as it takes to clear up enough space for this new image.
@@ -157,7 +157,7 @@ impl ImageDownloadHandler {
157157
.bind(image_config.id)
158158
.bind(
159159
(images_dir_size as u64)
160-
.saturating_add(image_download_size)
160+
.saturating_add(image_config.artifact_size_bytes)
161161
.saturating_sub(ctx.config().images.max_cache_size()) as i64,
162162
)
163163
.fetch_all(&mut *tx)
@@ -202,7 +202,7 @@ impl ImageDownloadHandler {
202202

203203
metrics::IMAGE_CACHE_COUNT.set(cache_count + 1 - removed_count);
204204
metrics::IMAGE_CACHE_SIZE
205-
.set(images_dir_size + image_download_size as i64 - removed_bytes);
205+
.set(images_dir_size + image_config.artifact_size_bytes as i64 - removed_bytes);
206206

207207
sqlx::query(indoc!(
208208
"
@@ -517,53 +517,6 @@ impl ImageDownloadHandler {
517517

518518
Ok(addresses)
519519
}
520-
521-
/// Attempts to fetch HEAD for the image download url and determine the image's download size.
522-
async fn fetch_image_download_size(
523-
&self,
524-
ctx: &Ctx,
525-
image_config: &protocol::Image,
526-
) -> Result<u64> {
527-
let addresses = self.get_image_addresses(ctx, image_config).await?;
528-
529-
let mut iter = addresses.into_iter();
530-
while let Some(artifact_url) = iter.next() {
531-
// Log the full URL we're attempting to download from
532-
tracing::info!(image_id=?image_config.id, %artifact_url, "attempting to download image");
533-
534-
match reqwest::Client::new()
535-
.head(&artifact_url)
536-
.send()
537-
.await
538-
.and_then(|res| res.error_for_status())
539-
{
540-
Ok(res) => {
541-
tracing::info!(image_id=?image_config.id, %artifact_url, "successfully fetched image HEAD");
542-
543-
// Read Content-Length header from response
544-
let image_size = res
545-
.headers()
546-
.get(reqwest::header::CONTENT_LENGTH)
547-
.context("no Content-Length header")?
548-
.to_str()?
549-
.parse::<u64>()
550-
.context("invalid Content-Length header")?;
551-
552-
return Ok(image_size);
553-
}
554-
Err(err) => {
555-
tracing::warn!(
556-
image_id=?image_config.id,
557-
%artifact_url,
558-
%err,
559-
"failed to fetch image HEAD"
560-
);
561-
}
562-
}
563-
}
564-
565-
bail!("artifact url could not be resolved");
566-
}
567520
}
568521

569522
/// Parses total bytes read from tar output.

0 commit comments

Comments
 (0)