Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 32 additions & 8 deletions go/store/blobstore/git_blobstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
54 changes: 43 additions & 11 deletions go/store/blobstore/git_blobstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions go/store/nbs/bs_persister.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
3 changes: 3 additions & 0 deletions go/store/nbs/no_conjoin_bs_persister.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading