go/store/nbs: store.go: Fix a bug which caused dolt gc --full to not collect new data sometimes.

If `dolt gc --full` was run, and then branches were changed and `dolt gc` was run, possibly multiple times, and then all of the state was returned to the state of the database during the initial `dolt gc --full`, then running `dolt gc --full` anew would falsely conclude that there was nothing to collect.

After the GC work is done, and we are inside swapTables, there is no need to check for whether we should swap the tables. We already have done the work to compute the new files and we should apply them regardless.
This commit is contained in:
Aaron Son
2025-10-23 10:54:35 -07:00
parent cfa982c420
commit f3adba56cc
2 changed files with 39 additions and 7 deletions

View File

@@ -2255,13 +2255,6 @@ func (nbs *NomsBlockStore) swapTables(ctx context.Context, specs []tableSpec, mo
specs: specs,
}
sameGcGen := newContents.gcGen == nbs.upstream.gcGen
hasLocalNovelty := nbs.hasLocalGCNovelty()
if sameGcGen && !hasLocalNovelty {
// Nothing has changed. Bail early.
return nil
}
upstream, err := nbs.manifestMgr.UpdateGCGen(ctx, nbs.upstream.lock, newContents, nbs.stats, nil)
if err != nil {
return err

View File

@@ -571,6 +571,45 @@ SQL
fi
}
@test "garbage_collection: dolt gc --full reverting to a previous state after some intervening dolt gcs is NOT a no-op" {
mkdir -p one/two
cd one/two
dolt init
dolt gc --full
rm -rf .dolt/stats
ls -laR | grep -v '^d' > ../../files_after_first_full_gc
cp ./.dolt/noms/oldgen/manifest ../../oldgen_manifest_after_first_full_gc
dolt branch -c main test
dolt checkout test
dolt sql -q 'create table vals (id int primary key);'
dolt commit -Am 'commit schema'
dolt gc
dolt sql -q 'insert into vals values (1),(2),(3);'
dolt commit -Am 'commit values'
dolt gc
rm -rf .dolt/stats
ls -laR | grep -v '^d' > ../../files_after_nonfull_gc
cp ./.dolt/noms/oldgen/manifest ../../oldgen_manifest_after_nonfull_gc
dolt checkout main
dolt branch -D test
dolt gc --full
rm -rf .dolt/stats
ls -laR | grep -v '^d' > ../../files_after_last_full_gc
cp ./.dolt/noms/oldgen/manifest ../../oldgen_manifest_after_last_full_gc
# This should exit 0 because the gc should have changed things.
if cmp ../../files_after_nonfull_gc ../../files_after_last_full_gc; then
echo "expected dolt gc to change things, but it didn't."
diff ../../files_after_nonfull_gc ../../files_after_last_full_gc || true
false
fi
# This should exit 0 because the gc should have changed things.
if cmp ./.dolt/noms/oldgen/manifest ../../oldgen_manifest_after_nonfull_gc; then
echo "expected dolt gc --full after a dolt gc to change the manifest, updating the gcgen at least, but it didn't."
diff ./.dolt/noms/oldgen/manifest ../../oldgen_manifest_after_nonfull_gc || true
false
fi
}
@test "garbage_collection: dolt gc --archive-level not 0" {
dolt gc --archive-level 1
run dolt admin storage list