From aeae2f20a3233fb14cb4c49cd35fa57976ddda32 Mon Sep 17 00:00:00 2001
From: Trevor Bedford <trevor@bedford.io>
Date: Thu, 2 Jul 2026 07:23:12 -0700
Subject: [PATCH 1/3] upload-to-s3: read the source once instead of three times

upload-to-s3 read the uncompressed source three separate times before the
upload: once for the sha256sum stored as object metadata, once for `wc -l`
(the recordcount metadata), and once to compress it. For large inputs those
three reads -- at disk speed -- dominate the run, not the S3 transfer itself.

Read the source once instead: tee the single read to the hasher and the line
counter (via fifos) while compressing to a temp file, then upload that file.
Uploading a real file rather than piping to `aws s3 cp -` also lets the AWS
CLI parallelize the multipart transfer instead of a single slow stdin stream.

Measured in nextstrain/ncov-ingest (GenBank, ~262 GB uncompressed sequences):
per-upload wall dropped ~3x on the large read-bound files (e.g.
sequences.fasta.zst 8131s -> 1739s), ~3.15x summed across all upload rules,
and ~72 min off the end-to-end run (the big uploads are on the critical path).

The uploaded object is unchanged: same body, same sha256sum/recordcount
metadata (the hash is still of the uncompressed source), same content-type.
Temp artifacts are removed on any exit via a trap.

Behaviour note: the sha256 is now computed while compressing, so the "files
are identical, skipping upload" short-circuit can no longer avoid the
read+compress -- it only skips the upload (the head-object check still runs
first, so nothing is transferred when unchanged).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 scripts/upload-to-s3 | 60 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 46 insertions(+), 14 deletions(-)

diff --git a/scripts/upload-to-s3 b/scripts/upload-to-s3
index 36d171c..d032527 100755
--- a/scripts/upload-to-s3
+++ b/scripts/upload-to-s3
@@ -3,6 +3,12 @@ set -euo pipefail
 
 bin="$(dirname "$0")"
 
+# Temp artifacts, cleaned up however the script exits. Globals so the EXIT trap
+# sees them regardless of where in main() we leave (return, `exit`, or set -e).
+tmp=""
+workdir=""
+trap '[[ -n "$workdir" ]] && rm -rf "$workdir"; [[ -n "$tmp" ]] && rm -f "$tmp"' EXIT
+
 main() {
     local quiet=0
 
@@ -24,24 +30,50 @@ main() {
     local bucket="${s3path%%/*}"
     local key="${s3path#*/}"
 
-    local src_hash dst_hash no_hash=0000000000000000000000000000000000000000000000000000000000000000
-    src_hash="$("$bin/sha256sum" < "$src")"
-    dst_hash="$(aws s3api head-object --bucket "$bucket" --key "$key" --query Metadata.sha256sum --output text 2>/dev/null || echo "$no_hash")"
+    # Read $src exactly once: while compressing it to a temp file, tee the same
+    # bytes to the hasher and the line counter. The old code read $src three
+    # separate times (sha256sum, wc -l, zstd); for the large aligned/sequences
+    # files those reads -- not the S3 transfer -- dominated the upload rules.
+    #
+    # Uploading a real file (rather than piping to `aws s3 cp -`) also lets the
+    # CLI parallelize the multipart transfer; a stdin stream can't seek and
+    # degrades to one slow stream. The temp lives next to $src (same big volume).
+    local upload_file="$src"
+    workdir="$(mktemp -d)"
 
-    if [[ $src_hash != "$dst_hash" ]]; then
-        # The record count may have changed
+    local -a compressor=()
+    case "$dst" in
+        *.gz)  compressor=(gzip -c) ;;
+        *.xz)  compressor=(xz -2 -T0 -c) ;;
+        *.zst) compressor=(zstd -T0 -c) ;;
+    esac
+
+    local src_hash src_record_count
+    if [[ ${#compressor[@]} -gt 0 ]]; then
+        tmp="$(mktemp "$src.upload.XXXXXX")"
+        upload_file="$tmp"
+        mkfifo "$workdir/hash.fifo" "$workdir/count.fifo"
+        "$bin/sha256sum" < "$workdir/hash.fifo" > "$workdir/hash" &
+        local hash_pid=$!
+        wc -l < "$workdir/count.fifo" > "$workdir/count" &
+        local count_pid=$!
+        tee "$workdir/hash.fifo" "$workdir/count.fifo" < "$src" | "${compressor[@]}" > "$tmp"
+        wait "$hash_pid" "$count_pid"
+        src_hash="$(cat "$workdir/hash")"
+        src_record_count="$(cat "$workdir/count")"
+    else
+        # No compression (e.g. the small *.json files) -- hash and count directly.
+        src_hash="$("$bin/sha256sum" < "$src")"
         src_record_count="$(wc -l < "$src")"
+    fi
+
+    local dst_hash no_hash=0000000000000000000000000000000000000000000000000000000000000000
+    dst_hash="$(aws s3api head-object --bucket "$bucket" --key "$key" --query Metadata.sha256sum --output text 2>/dev/null || echo "$no_hash")"
 
+    if [[ $src_hash != "$dst_hash" ]]; then
         echo "Uploading $src → $dst"
-        if [[ "$dst" == *.gz ]]; then
-            gzip -c "$src"
-        elif  [[ "$dst" == *.xz ]]; then
-            xz -2 -T0 -c "$src"
-        elif [[ "$dst" == *.zst ]]; then
-            zstd -T0 -c "$src"
-        else
-            cat "$src"
-        fi | aws s3 cp --no-progress - "$dst" --metadata sha256sum="$src_hash",recordcount="$src_record_count" "$(content-type "$dst")"
+
+        aws s3 cp --no-progress "$upload_file" "$dst" --metadata sha256sum="$src_hash",recordcount="$src_record_count" "$(content-type "$dst")"
 
         if [[ -n $cloudfront_domain ]]; then
             echo "Creating CloudFront invalidation for $cloudfront_domain/$key"

From 074429a44226b4fa1554c65e7045c789e9dacea1 Mon Sep 17 00:00:00 2001
From: Trevor Bedford <trevor@bedford.io>
Date: Thu, 2 Jul 2026 15:02:14 -0700
Subject: [PATCH 2/3] upload-to-s3: wait on the hash and count pids separately

`wait a b` returns only b's exit status, so a failure of the sha256 process
was masked from `set -e`: the script could proceed and upload with an empty
or wrong sha256sum in the object metadata. Wait for each pid on its own line
so either failure aborts.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 scripts/upload-to-s3 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/upload-to-s3 b/scripts/upload-to-s3
index d032527..10b84cb 100755
--- a/scripts/upload-to-s3
+++ b/scripts/upload-to-s3
@@ -58,7 +58,10 @@ main() {
         wc -l < "$workdir/count.fifo" > "$workdir/count" &
         local count_pid=$!
         tee "$workdir/hash.fifo" "$workdir/count.fifo" < "$src" | "${compressor[@]}" > "$tmp"
-        wait "$hash_pid" "$count_pid"
+        # Wait one at a time: `wait a b` returns only b's status, so a failing
+        # hasher would be masked from set -e (uploading with a bad sha256sum).
+        wait "$hash_pid"
+        wait "$count_pid"
         src_hash="$(cat "$workdir/hash")"
         src_record_count="$(cat "$workdir/count")"
     else

From f1d9713290a62dfff87a1d32b5b72400effd5a07 Mon Sep 17 00:00:00 2001
From: Trevor Bedford <trevor@bedford.io>
Date: Thu, 2 Jul 2026 15:08:33 -0700
Subject: [PATCH 3/3] upload-to-s3: fail early if the source file is unreadable

If $src can't be read, `tee < "$src"` fails and set -e aborts -- but the
background sha256/wc readers, already blocked opening their fifos, never get a
writer and leak (hang). Guard with `[[ -r "$src" ]]` before launching them,
which also gives a clear error message.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 scripts/upload-to-s3 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/upload-to-s3 b/scripts/upload-to-s3
index 10b84cb..e327e3e 100755
--- a/scripts/upload-to-s3
+++ b/scripts/upload-to-s3
@@ -26,6 +26,11 @@ main() {
     local dst="${2:?A destination s3:// URL is required as the second argument.}"
     local cloudfront_domain="${3:-}"
 
+    # Fail early if $src is unreadable. Otherwise `tee < "$src"` below fails, but
+    # the background hasher/counter -- already blocked opening their fifos -- get
+    # no writer and hang instead of being cleaned up.
+    [[ -r "$src" ]] || { echo "upload-to-s3: cannot read source file: $src" >&2; exit 1; }
+
     local s3path="${dst#s3://}"
     local bucket="${s3path%%/*}"
     local key="${s3path#*/}"