From c21254dc3d3a8edfaa401b026cd9de53751f6043 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Wed, 30 Sep 2026 11:17:19 -0700 Subject: [PATCH] Complete protocol v2 object-info support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? The server implemented an undiscoverable, protocol-incompatible `object-info` handler that decoded local objects and could not answer from remote-served pack bases. ### What? Advertise `object-info`, parse its post-delimiter arguments, emit the protocol-defined size response, and read headers through either local or remote object access. Add a manifest-versioned bounded size cache and parallelize remote misses across requested objects. The request path remains one conditional manifest GET. Warm cache hits and local header lookups add zero bucket reads; remote cache misses add only the pack range reads already required by `RemotePacks::header`, issued in parallel across requested objects. No CAS or write rate changes. ## Test Plan ✅ `cargo check -p walgit-server` ✅ `cargo test -p walgit-git read_object_info_args_reads_size_and_oids_after_delimiter` ✅ `cargo test -p walgit-server --test e2e info_refs_v2_advertises_capabilities -- --exact` ✅ `cargo test -p walgit-server --test e2e object_info_v2_reports_object_size -- --exact` ✅ `cargo fmt --all -- --check` ✅ `git diff --check` ## Issue Closes #80 --- crates/walgit-git/src/pkt.rs | 43 +++++++++++++++-- crates/walgit-server/src/cache.rs | 61 ++++++++++++++++++++++++ crates/walgit-server/src/smart.rs | 78 +++++++++++++++++++++++++++---- crates/walgit-server/tests/e2e.rs | 52 +++++++++++++++++++++ docs/ROUNDTRIPS.md | 1 + 5 files changed, 224 insertions(+), 11 deletions(-) diff --git a/crates/walgit-git/src/pkt.rs b/crates/walgit-git/src/pkt.rs index ab7c3a89..9adf8ea2 100644 --- a/crates/walgit-git/src/pkt.rs +++ b/crates/walgit-git/src/pkt.rs @@ -358,19 +358,43 @@ pub async fn read_ls_refs_args( /// Arguments for the v2 `object-info` command. #[derive(Debug, Default, Clone)] pub struct ObjectInfoRequest { + pub size: bool, pub oids: Vec, } pub fn parse_object_info(cmd: &V2Command) -> ObjectInfoRequest { let mut req = ObjectInfoRequest::default(); for a in &cmd.args { - if let Some(o) = a.strip_prefix("oid ") { - req.oids.push(o.to_string()); - } + parse_object_info_line(&mut req, a); } req } +fn parse_object_info_line(req: &mut ObjectInfoRequest, line: &str) { + if line == "size" { + req.size = true; + } else if let Some(oid) = line.strip_prefix("oid ") { + req.oids.push(oid.to_string()); + } +} + +/// Read the command-specific section after the v2 command delimiter. +pub async fn read_object_info_args( + mut reader: R, + mut req: ObjectInfoRequest, +) -> Result { + loop { + match read_pkt_line(&mut reader).await? { + Some(PktLine::Data(data)) => { + parse_object_info_line(&mut req, String::from_utf8_lossy(&data).trim_end()); + } + Some(PktLine::Delim) => {} + Some(PktLine::Flush | PktLine::ResponseEnd) | None => break, + } + } + Ok(req) +} + fn io_to_git(e: std::io::Error) -> GitError { GitError::Io(e) } @@ -447,4 +471,17 @@ mod tests { assert_eq!(req.prefixes, vec!["refs/heads/ref-199"]); assert!(req.peel); } + + #[tokio::test] + async fn read_object_info_args_reads_size_and_oids_after_delimiter() { + let mut body = Vec::new(); + encode_data(&mut body, b"size\n"); + encode_data(&mut body, b"oid 1111111111111111111111111111111111111111\n"); + encode_flush(&mut body); + let req = read_object_info_args(std::io::Cursor::new(body), ObjectInfoRequest::default()) + .await + .unwrap(); + assert!(req.size); + assert_eq!(req.oids, vec!["1111111111111111111111111111111111111111"]); + } } diff --git a/crates/walgit-server/src/cache.rs b/crates/walgit-server/src/cache.rs index 36346480..1418c793 100644 --- a/crates/walgit-server/src/cache.rs +++ b/crates/walgit-server/src/cache.rs @@ -245,6 +245,65 @@ impl RefIndexCache { } } +// --------------------------------------------------------------------------- +// Object information (protocol v2) +// --------------------------------------------------------------------------- + +#[derive(Clone, PartialEq, Eq, Hash)] +struct ObjectInfoKey { + repo: String, + version: String, + oid: gix_hash::ObjectId, +} + +/// Bounded cache of object sizes. The manifest version keeps negative results +/// from surviving a push that makes an object visible. +#[derive(Clone)] +pub struct ObjectInfoCache { + inner: Cache, +} + +impl ObjectInfoCache { + pub fn new(max_entries: usize) -> Self { + Self { + inner: Cache::builder().max_capacity(max_entries as u64).build(), + } + } + + fn key(repo: &str, version: Option<&Version>, oid: gix_hash::ObjectId) -> ObjectInfoKey { + ObjectInfoKey { + repo: repo.to_string(), + version: version.map(|v| v.as_str().to_string()).unwrap_or_default(), + oid, + } + } + + pub fn get( + &self, + repo: &str, + version: Option<&Version>, + oid: gix_hash::ObjectId, + ) -> Option { + let value = self.inner.get(&Self::key(repo, version, oid)); + if value.is_some() { + metrics::counter!("walgit_cache_object_info_hit").increment(1); + } else { + metrics::counter!("walgit_cache_object_info_miss").increment(1); + } + value + } + + pub fn insert( + &self, + repo: &str, + version: Option<&Version>, + oid: gix_hash::ObjectId, + size: i64, + ) { + self.inner.insert(Self::key(repo, version, oid), size); + } +} + // --------------------------------------------------------------------------- // ServerCaches — aggregate held by AppState // --------------------------------------------------------------------------- @@ -254,6 +313,7 @@ impl RefIndexCache { pub struct ServerCaches { pub ref_advert: RefAdvertCache, pub ref_index: RefIndexCache, + pub object_info: ObjectInfoCache, /// Rendered sha-addressed web API JSON (immutable): key = repo\0kind\0sha\0path. pub api_immutable: Cache, } @@ -263,6 +323,7 @@ impl ServerCaches { Self { ref_advert: RefAdvertCache::new(cfg.cache.ref_advert_entries), ref_index: RefIndexCache::new(cfg.cache.ref_advert_entries.max(64)), + object_info: ObjectInfoCache::new(cfg.cache.object_info_entries), api_immutable: Cache::builder() .max_capacity(64 * 1024 * 1024) .weigher(|k: &String, v: &bytes::Bytes| { diff --git a/crates/walgit-server/src/smart.rs b/crates/walgit-server/src/smart.rs index ca89d49c..21375037 100644 --- a/crates/walgit-server/src/smart.rs +++ b/crates/walgit-server/src/smart.rs @@ -151,6 +151,7 @@ async fn v2_capability_advert( // packfile section: auth, WAL sync, materialization progress. Both engines frame their sections that way. fetch.push_str(" sideband-all packfile-uris packfile-indexes"); pktline::encode_text(buf, &format!("{fetch}\n")); + pktline::encode_text(buf, "object-info\n"); pktline::encode_text(buf, "server-option\n"); let fmt = match handle.local().object_format() { walgit_git::ObjectFormat::Sha1 => "sha1", @@ -333,16 +334,77 @@ async fn upload_pack_v2( )) } "object-info" => { - let _guard = handle.sync().await.map_err(wal_err)?; let req = walgit_git::pkt::parse_object_info(&cmd); + let req = walgit_git::pkt::read_object_info_args(reader, req) + .await + .map_err(git_err)?; + if !req.size { + return Err(ApiError::BadRequest( + "object-info currently requires the size attribute".into(), + )); + } + let (_guard, access) = handle.sync_objects().await.map_err(wal_err)?; + let repo_key = route.id.to_string(); + let version = handle.manifest_version(); + let oids: Vec> = req + .oids + .iter() + .map(|hex| gix_hash::ObjectId::from_hex(hex.as_bytes()).ok()) + .collect(); + let sizes = match access { + walgit_wal::ObjectAccess::Local => { + let repo = handle.local().gix(); + oids.iter() + .map(|oid| { + let Some(oid) = oid else { return -1 }; + if let Some(size) = + st.caches.object_info.get(&repo_key, version.as_ref(), *oid) + { + return size; + } + let size = gix_object::FindHeader::try_header(&repo.objects, oid) + .ok() + .flatten() + .map_or(-1, |header| { + i64::try_from(header.size).unwrap_or(i64::MAX) + }); + st.caches + .object_info + .insert(&repo_key, version.as_ref(), *oid, size); + size + }) + .collect() + } + walgit_wal::ObjectAccess::Remote(packs) => { + let results = futures::future::join_all(oids.iter().map(|oid| { + let packs = packs.clone(); + let cache = st.caches.object_info.clone(); + let repo_key = repo_key.clone(); + let version = version.clone(); + async move { + let Some(oid) = oid else { return Ok(-1) }; + if let Some(size) = cache.get(&repo_key, version.as_ref(), *oid) { + return Ok(size); + } + let size = packs + .header(oid) + .await? + .map_or(-1, |(_, size)| i64::try_from(size).unwrap_or(i64::MAX)); + cache.insert(&repo_key, version.as_ref(), *oid, size); + Ok::(size) + } + })) + .await; + results + .into_iter() + .collect::, _>>() + .map_err(wal_err)? + } + }; let mut sizes_buf = Vec::with_capacity(256); - let repo = handle.local().gix(); - for hex in &req.oids { - let size = gix_hash::ObjectId::from_hex(hex.as_bytes()) - .ok() - .and_then(|oid| repo.find_object(oid).ok()) - .map_or(-1, |o| o.data.len() as i64); - pktline::encode_text(&mut sizes_buf, &format!("size {size}\n")); + pktline::encode_text(&mut sizes_buf, "size\n"); + for (hex, size) in req.oids.iter().zip(sizes) { + pktline::encode_text(&mut sizes_buf, &format!("{hex} {size}\n")); } pktline::encode_flush(&mut sizes_buf); Ok(text_response( diff --git a/crates/walgit-server/tests/e2e.rs b/crates/walgit-server/tests/e2e.rs index b7cc4441..c92b07f8 100644 --- a/crates/walgit-server/tests/e2e.rs +++ b/crates/walgit-server/tests/e2e.rs @@ -30,10 +30,62 @@ async fn info_refs_v2_advertises_capabilities() -> TestResult { assert!(out.contains("version 2")); assert!(out.contains("ls-refs=unborn")); assert!(out.contains("fetch=shallow wait-for-done")); + assert!(out.contains("object-info")); assert!(!out.contains("bundle-uri"), "{out}"); Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn object_info_v2_reports_object_size() -> TestResult { + let server = Server::start().await?; + server.put_repo("t", "object-info").await?; + let src = TestRepo::synthetic(1, 1)?; + git_in( + &src, + &[ + "remote", + "add", + "origin", + &server.repo_url("t", "object-info"), + ], + )?; + git_in(&src, &["push", "-q", "origin", "main"])?; + let oid = git_in(&src, &["rev-parse", "main"])?; + let size = git_in(&src, &["cat-file", "-s", oid.trim()])?; + + let mut body = Vec::new(); + walgit_server::pktline::encode_text(&mut body, "command=object-info\n"); + walgit_server::pktline::encode_delim(&mut body); + walgit_server::pktline::encode_text(&mut body, "size\n"); + walgit_server::pktline::encode_text(&mut body, &format!("oid {}\n", oid.trim())); + walgit_server::pktline::encode_flush(&mut body); + + let response = reqwest::Client::new() + .post(format!( + "{}/t/object-info.git/git-upload-pack", + server.base_url + )) + .header("Git-Protocol", "version=2") + .header( + reqwest::header::CONTENT_TYPE, + "application/x-git-upload-pack-request", + ) + .body(body) + .send() + .await?; + assert!(response.status().is_success()); + let response = response.bytes().await?; + let expected = format!("{} {}\n", oid.trim(), size.trim()); + assert!( + response + .windows(expected.len()) + .any(|window| window == expected.as_bytes()), + "{}", + String::from_utf8_lossy(&response) + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn push_clone_roundtrip_v2() -> TestResult { let server = Server::start().await?; diff --git a/docs/ROUNDTRIPS.md b/docs/ROUNDTRIPS.md index 447af901..622f8ea1 100644 --- a/docs/ROUNDTRIPS.md +++ b/docs/ROUNDTRIPS.md @@ -47,6 +47,7 @@ right shape. This document is the thinking tool; apply it to every protocol chan | Conserving lifecycle | acquire lease → captured refs/full sync → each output publishes additively → final seal (lease heartbeat → refs revalidation → immutable log PUT → manifest CAS) | one additive publication per new physical checksum and one final CAS; heartbeat GET/CAS calls and full-sync downloads are additional maintenance costs; no change to ordinary push/read budgets | `pack_lifecycle.rs`, `classification.rs::seal_pack_replacement` | | Coverage repair | captured snapshot Create → bounded batches of classification CAS → final certificate CAS | snapshot equality GET only on Create conflict; up to 128 classifications per batch; every certificate rechecks current policy and live membership | `pack_lifecycle.rs::repair_metadata` | | Native URI selection | existing fetch sync → exact coverage snapshot GET | normally +1 GET, shared maximum 3 attempts across all groups; current manifest/ref view capture adds no GET; optional index trailers are local bounded reads | `packfile_uri.rs::select`, `snapshots.rs::fetch_view` | +| Protocol v2 object-info | 1 conditional manifest GET; warm size cache adds 0 reads; a remote miss performs the object's pack range reads, with requested objects in parallel | no LIST or pack materialization; local objects use header lookup and remote delta sizes may require delta/base headers | `smart.rs`, `cache.rs`, `remote.rs::header` | | Static pack/index GET | manifest revalidation → object HEAD → body/range GET | 3 warm requests; conditional 304/HEAD omits the body GET; cold registry open may add its normal snapshot/tail reads; no pack materialization or LIST | `packfile_uri.rs::get`, `static_object.rs` | | Checkpoint | 1 cond GET (freshness) → refs PUT ∥ checkpoint PUT → manifest CAS | 3 rounds, 4 requests (was 6/6 until 2026-08-22: a bundle-list GET before the checkpoint PUT and a log GET for provenance times sat in the chain; times now come from the writer's own applied state, `bundle_key` is no longer looked up) | `checkpoint.rs` | | Validated remote index admission | warm valid index: 0 store requests; absent/corrupt index: index GET (HEAD first only for old descriptors with unknown size), at most four indexes downloading concurrently | unchanged healthy store depth; corruption now triggers repair instead of admitting bad evidence. Checksums, lock, mmap and cleanup are local work; no LIST. Per-index checksum CPU cost occurs when constructing a new remote inventory, not per object lookup. | `index_cache.rs`, `remote.rs` |