Skip upfront source fetch when relay handles the push · Entire

Skip upfront source fetch when relay handles the push

3ca6a57→main·

Soph·2mo ago·2 files·+142 added/-4 removed

The sync flow fetched the full source closure into an in-memory store before deciding which strategy would run. For relay-only paths the strategy then fetched the same closure again from source to stream to the target — every relay-eligible sync paid for the source pack twice.

Skip the upfront FetchToStore when:

- Force and prune are off (otherwise relay is disabled and materialized needs the closure). - No desired ref already exists on target at a different hash (otherwise BuildPlans needs ancestry data for fast-forward detection, or the divergent ref will route to materialized).

In that case all plans are skips, creates, or tag-creates — incremental relay handles them via streaming FetchPack and the local store goes unread. The conservative check leaves the prior path in place for any case that might actually need the closure (force, prune, FF updates, tag retargets).

Verified end-to-end: TestRun_IntegrationSkipsLocalFetchOnRelayOnlySync asserts exactly one upload-pack fetch on a target-has-master / source-adds-release scenario, and TestRun_IntegrationKeepsLocalFetchWhenAncestryNeeded ensures the FF-update path still has the store populated for ReachesCommit.

Sessions

a64da795353eView transcript

Changes

2

232 unmodified lines

// TestRun_IntegrationSkipsLocalFetchOnRelayOnlySync verifies the
// double-fetch optimization: when every desired ref is a skip or a create
// (no FF check needed, no force, no prune), the upfront FetchToStore that
// populates the in-memory store is skipped entirely. The incremental relay
// still does its own FetchPack to stream to target, so we end up with one
// source upload-pack call instead of two.
func TestRun_IntegrationSkipsLocalFetchOnRelayOnlySync(t *testing.T) {
    sourceRepo, sourceFS := newSourceRepo(t)
    makeCommits(t, sourceRepo, sourceFS, 3)

targetRepo, err := git.Init(memory.NewStorage())
    if err != nil {
        t.Fatalf("init target repo: %v", err)
    }
    if err := copyRefsAndObjects(sourceRepo.Storer, targetRepo.Storer, []plumbing.ReferenceName{plumbing.NewBranchReferenceName(testBranch)}); err != nil {
        t.Fatalf("copy target baseline: %v", err)
    }

sourceHead, err := sourceRepo.Reference(plumbing.NewBranchReferenceName(testBranch), true)
    if err != nil {
        t.Fatalf("resolve source head: %v", err)
    }
    releaseRef := plumbing.NewBranchReferenceName("release")
    if err := sourceRepo.Storer.SetReference(plumbing.NewHashReference(releaseRef, sourceHead.Hash())); err != nil {
        t.Fatalf("set source release branch: %v", err)
    }

sourceServer := newSmartHTTPRepoServerV2(t, sourceRepo)
    targetServer := newSmartHTTPRepoServer(t, targetRepo)
    targetServer.receivePackThinCap = true
    defer sourceServer.Close()
    defer targetServer.Close()

result, err := Run(context.Background(), Config{
        Source:       Endpoint{URL: sourceServer.RepoURL()},
        Target:       Endpoint{URL: targetServer.RepoURL()},
        ProtocolMode: protocolModeAuto,
    })
    if err != nil {
        t.Fatalf("sync: %v", err)
    }
    if !result.Relay || result.RelayMode != relayModeIncremental {
        t.Fatalf("expected incremental relay, got mode=%q reason=%q relay=%v", result.RelayMode, result.RelayReason, result.Relay)
    }

if got := sourceServer.Wants(serviceUploadPack, metricPack); got != 1 {
        t.Fatalf("expected 1 want total (single relay fetch), got %d — likely indicates the upfront FetchToStore was not skipped", got)
    }
}

// TestRun_IntegrationKeepsLocalFetchWhenAncestryNeeded ensures the fetch is // still performed for a fast-forward update where BuildPlans calls // ReachesCommit on the local store. Skipping it would crash the planner. func TestRun_IntegrationKeepsLocalFetchWhenAncestryNeeded(t *testing.T) { sourceRepo, sourceFS := newSourceRepo(t) makeCommits(t, sourceRepo, sourceFS, 2)

targetRepo, _ := newSourceRepo(t)

sourceServer := newSmartHTTPRepoServer(t, sourceRepo) targetServer := newSmartHTTPRepoServer(t, targetRepo) targetServer.receivePackThinCap = true defer sourceServer.Close() defer targetServer.Close()

cfg := Config{ Source: Endpoint{URL: sourceServer.RepoURL()}, Target: Endpoint{URL: targetServer.RepoURL()}, }

if _, err := Run(context.Background(), cfg); err != nil { t.Fatalf("seed sync: %v", err) }

// Advance source so the resync plans an FF update. makeCommits(t, sourceRepo, sourceFS, 1)

result, err := Run(context.Background(), cfg) if err != nil { t.Fatalf("resync: %v", err) } if !result.Relay || result.RelayMode != relayModeIncremental { t.Fatalf("expected incremental relay, got mode=%q reason=%q", result.RelayMode, result.RelayReason) } if result.Pushed != 1 { t.Fatalf("expected 1 ref pushed, got %+v", result) } }


--- Session setup (issue #12) ---

// syncSession holds the shared state for a sync operation, reducing
// bandwidth needed. Normal sync: allocate in-memory repo and fetch objects
// Normal sync: allocate in-memory repo. The source closure is fetched
// lazily — only when planning needs ancestry data (FF detection on a divergent branch) or the materialized fallback will run (force, prune, or any divergent ref). Pure skip/create plans take incremental relay without ever decoding source objects locally, so the upfront fetch is a wasted full-pack round trip.
repo, err := git.Init(memory.NewStorage(), nil)
if err != nil {
    return Result{}, fmt.Errorf("init in-memory repository: %w", err)
}
gpDesired := convert.DesiredRefs(desiredRefs)
if err := sourceService.FetchToStore(ctx, repo.Storer, s.sourceConn, gpDesired, targetRefMap); err != nil {
    if !errors.Is(err, git.NoErrAlreadyUpToDate) {
        return Result{}, fmt.Errorf("fetch to store: %w", err)
    }
}