diff --git a/go/store/blobstore/git_blobstore.go b/go/store/blobstore/git_blobstore.go index 10063b6fe07..5a3cf83285b 100644 --- a/go/store/blobstore/git_blobstore.go +++ b/go/store/blobstore/git_blobstore.go @@ -524,6 +524,34 @@ func (gbs *GitBlobstore) CleanupOwnedLocalRef(ctx context.Context) error { return err } +// Close best-effort deletes this instance's UUID-owned refs. +func (gbs *GitBlobstore) Close() error { + ctx := context.Background() + + gbs.mu.Lock() + defer gbs.mu.Unlock() + + deleteIfExists := func(ref string) error { + if ref == "" { + return nil + } + _, ok, err := gbs.api.TryResolveRefCommit(ctx, ref) + if err != nil { + return err + } + if !ok { + return nil + } + _, err = gbs.runner.Run(ctx, git.RunOptions{}, "update-ref", "-d", ref) + return err + } + + return errors.Join( + deleteIfExists(gbs.localRef), + deleteIfExists(gbs.remoteTrackingRef), + ) +} + func (gbs *GitBlobstore) syncForRead(ctx context.Context) error { if err := gbs.validateRemoteManaged(); err != nil { return err @@ -1034,15 +1062,11 @@ func (gbs *GitBlobstore) buildCommitForKeyWrite(ctx context.Context, parent git. return "", err } - var parentPtr *git.OID - if hasParent && parent != "" { - p := parent - parentPtr = &p - } - - commitOID, err := gbs.api.CommitTree(ctx, treeOID, parentPtr, msg, gbs.identity) + // Snapshot-only semantics: create commits with no parent so old snapshots become unreachable + // once refs advance (enables pruning / avoids cache history growth). + commitOID, err := gbs.api.CommitTree(ctx, treeOID, nil, msg, gbs.identity) if err != nil && gbs.identity == nil && isMissingGitIdentityErr(err) { - commitOID, err = gbs.api.CommitTree(ctx, treeOID, parentPtr, msg, defaultGitBlobstoreIdentity()) + commitOID, err = gbs.api.CommitTree(ctx, treeOID, nil, msg, defaultGitBlobstoreIdentity()) } if err != nil { return "", err diff --git a/go/store/blobstore/git_blobstore_test.go b/go/store/blobstore/git_blobstore_test.go index 80a0359552e..cba2a0cf255 100644 --- a/go/store/blobstore/git_blobstore_test.go +++ b/go/store/blobstore/git_blobstore_test.go @@ -282,6 +282,43 @@ func TestGitBlobstore_CleanupOwnedLocalRef_DeletesRef(t *testing.T) { require.Equal(t, bs.localRef, rnf.Ref) } +func TestGitBlobstore_Close_DeletesOwnedLocalAndTrackingRefs(t *testing.T) { + requireGitOnPath(t) + + ctx := context.Background() + remoteRepo, localRepo, localRunner := newRemoteAndLocalRepos(t, ctx) + _, err := remoteRepo.SetRefToTree(ctx, DoltDataRef, map[string][]byte{ + "manifest": []byte("seed\n"), + }, "seed") + require.NoError(t, err) + + bs, err := NewGitBlobstoreWithOptions(localRepo.GitDir, DoltDataRef, GitBlobstoreOptions{RemoteName: "origin"}) + require.NoError(t, err) + + // Force refs to be created via a remote-managed read. + ok, err := bs.Exists(ctx, "manifest") + require.NoError(t, err) + require.True(t, ok) + + localAPI := git.NewGitAPIImpl(localRunner) + _, err = localAPI.ResolveRefCommit(ctx, bs.localRef) + require.NoError(t, err) + _, err = localAPI.ResolveRefCommit(ctx, bs.remoteTrackingRef) + require.NoError(t, err) + + require.NoError(t, bs.Close()) + + _, err = localAPI.ResolveRefCommit(ctx, bs.localRef) + var rnf *git.RefNotFoundError + require.ErrorAs(t, err, &rnf) + require.Equal(t, bs.localRef, rnf.Ref) + + _, err = localAPI.ResolveRefCommit(ctx, bs.remoteTrackingRef) + rnf = nil + require.ErrorAs(t, err, &rnf) + require.Equal(t, bs.remoteTrackingRef, rnf.Ref) +} + func TestGitBlobstore_RemoteManaged_PutPushesToRemote(t *testing.T) { requireGitOnPath(t) @@ -439,14 +476,11 @@ func TestGitBlobstore_RemoteManaged_PutRetriesOnLeaseFailure(t *testing.T) { remoteHead, err := remoteAPI.ResolveRefCommit(ctx, DoltDataRef) require.NoError(t, err) - // Verify we rebuilt on top of the advanced remote head (i.e. parent is externalHead). + // Snapshot-only semantics: new remote head should be a non-merge commit with no parent. if v := externalHead.Load(); v != nil { - wantParent := v.(git.OID) - out, err := remoteRunner.Run(ctx, git.RunOptions{}, "rev-parse", remoteHead.String()+"^") + out, err := remoteRunner.Run(ctx, git.RunOptions{}, "cat-file", "-p", remoteHead.String()) require.NoError(t, err) - require.Equal(t, wantParent.String(), string(bytes.TrimSpace(out))) - _, err = remoteRunner.Run(ctx, git.RunOptions{}, "rev-parse", remoteHead.String()+"^2") - require.Error(t, err) // not a merge commit + require.NotContains(t, string(out), "\nparent ") } oid, typ, err := remoteAPI.ResolvePathObject(ctx, remoteHead, "k") @@ -628,12 +662,10 @@ func TestGitBlobstore_RemoteManaged_PutOverwritesDivergedLocalRef_NoMergeCommit( remoteHeadAfter, err := remoteAPI.ResolveRefCommit(ctx, DoltDataRef) require.NoError(t, err) - // New remote head is a normal (non-merge) commit built on remoteHeadBefore. - out, err := remoteRunner.Run(ctx, git.RunOptions{}, "rev-parse", remoteHeadAfter.String()+"^") + // Snapshot-only semantics: new remote head should be a non-merge commit with no parent. + out, err := remoteRunner.Run(ctx, git.RunOptions{}, "cat-file", "-p", remoteHeadAfter.String()) require.NoError(t, err) - require.Equal(t, remoteHeadBefore.String(), string(bytes.TrimSpace(out))) - _, err = remoteRunner.Run(ctx, git.RunOptions{}, "rev-parse", remoteHeadAfter.String()+"^2") - require.Error(t, err) + require.NotContains(t, string(out), "\nparent ") // Local-only divergence should not be present on remote. _, _, err = remoteAPI.ResolvePathObject(ctx, remoteHeadAfter, "local") diff --git a/go/store/nbs/bs_persister.go b/go/store/nbs/bs_persister.go index 0333f7b728c..86e586b19a0 100644 --- a/go/store/nbs/bs_persister.go +++ b/go/store/nbs/bs_persister.go @@ -234,6 +234,9 @@ func (bsp *blobstorePersister) PruneTableFiles(ctx context.Context, keeper func( } func (bsp *blobstorePersister) Close() error { + if c, ok := bsp.bs.(io.Closer); ok { + return c.Close() + } return nil } diff --git a/go/store/nbs/no_conjoin_bs_persister.go b/go/store/nbs/no_conjoin_bs_persister.go index 126330dcfa6..811d8f55f78 100644 --- a/go/store/nbs/no_conjoin_bs_persister.go +++ b/go/store/nbs/no_conjoin_bs_persister.go @@ -87,6 +87,9 @@ func (bsp *noConjoinBlobstorePersister) PruneTableFiles(ctx context.Context, kee } func (bsp *noConjoinBlobstorePersister) Close() error { + if c, ok := bsp.bs.(io.Closer); ok { + return c.Close() + } return nil }