From 9981e38119b4f652a23d55e112deb27e135a68df Mon Sep 17 00:00:00 2001 From: Daniel Reynaud Date: Mon, 19 Oct 2026 13:01:04 -0700 Subject: [PATCH] http-fetch: trace received pack bytes on failed downloads --- Documentation/technical/packfile-uri.adoc | 14 ++++++++++ http-fetch.c | 31 +++++++++++++++++------ http.c | 10 ++++---- http.h | 1 + t/t5702-protocol-v2.sh | 25 +++++++++++++++--- 5 files changed, 65 insertions(+), 16 deletions(-) diff --git a/Documentation/technical/packfile-uri.adoc b/Documentation/technical/packfile-uri.adoc index 3a65a92dc43793..42b3937f251609 100644 --- a/Documentation/technical/packfile-uri.adoc +++ b/Documentation/technical/packfile-uri.adoc @@ -70,6 +70,20 @@ fail without retrying. Interrupted pack transfers are not retried. HTTP 429 uses `http.maxRetries`, `http.retryAfter`, and `http.maxRetryTime` (disabled by default). +Each `http-fetch` process emits Trace2 `data` events in category `http-fetch` +with key `pack/received_bytes`. This is a cumulative count of PACK response +payload received during this invocation, starting at zero. It excludes an +existing partial file, HTTP error bodies, headers, and local indexing reads. +Bytes count when received, even if writing them to disk fails. Retries do not +reset the count. Quiet and serial downloads emit the same measurements. + +Snapshots are emitted during transfer no more often than twice per second. +When the HTTP operation returns, including on failure, a final snapshot precedes +`pack/received_bytes_complete=1`. Completion describes the measurement, not +download success. If a process is killed before that marker, the last snapshot +is only a lower bound. Consumers should take the last count per process, not +sum its snapshots. These events contain no URL or credentials. + When the client downloads the given URIs, it should store them with "keep" files, just like it does with the packfile in the `packfile` section. These additional "keep" files can only be removed after the refs have been updated - diff --git a/http-fetch.c b/http-fetch.c index 920a1ff1b81f6d..96f986cd74fc50 100644 --- a/http-fetch.c +++ b/http-fetch.c @@ -56,8 +56,15 @@ static int fetch_using_walker(const char *raw_url, int get_verbosely, struct packfile_progress { uint64_t bytes; uint64_t last_update; + int report_progress; }; +static void trace_packfile_progress(struct packfile_progress *progress) +{ + trace2_data_intmax("http-fetch", the_repository, + "pack/received_bytes", progress->bytes); +} + static void report_packfile_progress(struct packfile_progress *progress) { printf("bytes %"PRIu64"\n", progress->bytes); @@ -71,8 +78,13 @@ static void update_packfile_progress(void *data, size_t bytes) struct packfile_progress *progress = data; progress->bytes += bytes; - if (getnanotime() - progress->last_update >= 500000000) - report_packfile_progress(progress); + if (getnanotime() - progress->last_update >= 500000000) { + trace_packfile_progress(progress); + if (progress->report_progress) + report_packfile_progress(progress); + else + progress->last_update = getnanotime(); + } } static void fetch_single_packfile(struct object_id *packfile_hash, @@ -81,9 +93,11 @@ static void fetch_single_packfile(struct object_id *packfile_hash, int report_progress) { struct http_pack_request *preq; - struct packfile_progress progress = { 0 }; + struct packfile_progress progress = { .report_progress = report_progress }; int ret; + progress.last_update = getnanotime(); + trace_packfile_progress(&progress); http_init(NULL, url, 0); preq = new_direct_http_pack_request(packfile_hash->hash, xstrdup(url)); @@ -91,13 +105,14 @@ static void fetch_single_packfile(struct object_id *packfile_hash, die("couldn't create http pack request"); preq->index_pack_args = index_pack_args; preq->preserve_index_pack_stdout = 1; - if (report_progress) { - progress.last_update = getnanotime(); - preq->progress = update_packfile_progress; - preq->progress_data = &progress; - } + preq->progress = update_packfile_progress; + preq->progress_data = &progress; ret = run_http_pack_request(preq); + /* Preserve partial transfers before ordinary HTTP failures exit. */ + trace_packfile_progress(&progress); + trace2_data_intmax("http-fetch", the_repository, + "pack/received_bytes_complete", 1); if (ret != HTTP_START_FAILED) { if (ret != HTTP_OK) { struct url_info url; diff --git a/http.c b/http.c index e801bb20f44ed9..18b22d8ffeeb29 100644 --- a/http.c +++ b/http.c @@ -3036,17 +3036,17 @@ static size_t fwrite_http_pack(char *ptr, size_t size, size_t nmemb, void *data) { struct http_pack_request *preq = data; long code; - size_t written; + size_t bytes = st_mult(size, nmemb); if (curl_easy_getinfo(preq->slot->curl, CURLINFO_HTTP_CODE, &code) != CURLE_OK) return 0; /* Error bodies must not overwrite a partial pack shared with another fetch. */ if (code >= 300) - return size * nmemb; - written = fwrite(ptr, 1, st_mult(size, nmemb), preq->packfile); + return bytes; + /* Count received PACK payload even if the local write fails. */ if (preq->progress) - preq->progress(preq->progress_data, written); - return written; + preq->progress(preq->progress_data, bytes); + return fwrite(ptr, 1, bytes, preq->packfile); } int run_http_pack_request(struct http_pack_request *preq) diff --git a/http.h b/http.h index d3c33adec9b3de..0f6144f7f7fd3c 100644 --- a/http.h +++ b/http.h @@ -212,6 +212,7 @@ int http_get_info_packs(const char *base_url, const char *http_get_accept_language_header(void); struct http_pack_request { + /* Newly received PACK payload, excluding HTTP error bodies and resume offsets. */ void (*progress)(void *data, size_t bytes); void *progress_data; char *url; diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index 5f8d2dd3323a2e..c129c378930417 100755 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@ -1436,10 +1436,20 @@ fetch_pack () { --index-pack-arg=index-pack --index-pack-arg=--stdin "$pack_url" } +check_pack_received_bytes () { + grep "\"category\":\"http-fetch\".*\"key\":\"pack/received_bytes\"" pack-bytes.trace >pack-bytes-counts && + tail -n 1 pack-bytes-counts >pack-bytes-last && + test_grep "\"value\":\"$1\"" pack-bytes-last && + test_grep "\"category\":\"http-fetch\".*\"key\":\"pack/received_bytes_complete\".*\"value\":\"1\"" pack-bytes.trace +} + test_expect_success 'resumed packfile download survives two transient errors' ' setup_pack_retry 503 502 200 && cp pack-prefix "pack-retry-client/.git/objects/pack/pack-$pack_hash.pack.temp" && - fetch_pack && + >pack-bytes.trace && + GIT_TRACE2_EVENT="$TRASH_DIRECTORY/pack-bytes.trace" fetch_pack && + size=$(wc -c <"$pack") && + check_pack_received_bytes "$((size - 12))" && printf "GET|bytes=12-|\nGET|bytes=12-|\nGET|bytes=12-|\n" >expect && test_cmp expect "$HTTPD_ROOT_PATH/pack-retry.requests" && test_cmp "$pack" "pack-retry-client/.git/objects/pack/pack-$pack_hash.pack" @@ -1452,7 +1462,9 @@ do test_config_global http.maxRetries 3 && partial="pack-retry-client/.git/objects/pack/pack-$pack_hash.pack.temp" && cp pack-prefix "$partial" && - fetch_pack test_must_fail && + >pack-bytes.trace && + GIT_TRACE2_EVENT="$TRASH_DIRECTORY/pack-bytes.trace" fetch_pack test_must_fail && + check_pack_received_bytes 0 && test_cmp pack-prefix "$partial" && printf "GET|bytes=12-|\nGET|bytes=12-|\nGET|bytes=12-|\n" >expect && test_cmp expect "$HTTPD_ROOT_PATH/pack-retry.requests" @@ -1470,7 +1482,9 @@ done test_expect_success 'an interrupted response is not retried or truncated' ' setup_pack_retry truncated 200 && - fetch_pack test_must_fail && + >pack-bytes.trace && + GIT_TRACE2_EVENT="$TRASH_DIRECTORY/pack-bytes.trace" fetch_pack test_must_fail && + check_pack_received_bytes 12 && test_line_count = 1 "$HTTPD_ROOT_PATH/pack-retry.requests" && test_cmp pack-prefix "pack-retry-client/.git/objects/pack/pack-$pack_hash.pack.temp" ' @@ -1525,11 +1539,16 @@ test_expect_success 'parallel packfile URI fetch completes after HTTP 503 and 42 test_expect_success 'parallel URI progress respects quiet and no-progress' ' for option in --quiet --no-progress do + >pack-bytes.trace && + GIT_TRACE2_EVENT="$TRASH_DIRECTORY/pack-bytes.trace" \ GIT_TEST_SIDEBAND_ALL=1 \ git -c protocol.version=2 -c fetch.uriprotocols=http \ -c fetch.packfileUriJobs=2 clone "$option" \ "$HTTPD_URL/smart/http_parent" "http_child-$option" \ 2>no-ref-progress && + grep "\"category\":\"http-fetch\".*\"key\":\"pack/received_bytes_complete\".*\"value\":\"1\"" \ + pack-bytes.trace >pack-bytes-complete && + test_line_count = 3 pack-bytes-complete && test_grep ! -E "Fetching packs|Receiving objects|^bytes " no-ref-progress || return 1 done