br: MergeMigrations drops IngestedSstPaths from layers, causing storage leak of ext_backups

[RidRisR]

Bug Report

Summary

MergeMigrations() in br/pkg/stream/stream_metas.go only preserves IngestedSstPaths from m1 (the accumulated BASE), discarding m2 (the layer being merged). This causes processExtFullBackup() to never see layer-originated IngestedSstPaths, making the corresponding v1/ext_backups/ directories permanent orphans that are never cleaned up.

Code Location

br/pkg/stream/stream_metas.go, MergeMigrations():

func MergeMigrations(m1 *pb.Migration, m2 *pb.Migration) *pb.Migration {
    out := NewMigration()
    out.EditMeta = mergeMetaEdits(m1.GetEditMeta(), m2.GetEditMeta())
    out.Compactions = append(out.Compactions, m1.GetCompactions()...)
    out.Compactions = append(out.Compactions, m2.GetCompactions()...)       // both m1 and m2
    out.TruncatedTo = max(m1.GetTruncatedTo(), m2.GetTruncatedTo())
    out.DestructPrefix = append(out.DestructPrefix, m1.GetDestructPrefix()...)
    out.DestructPrefix = append(out.DestructPrefix, m2.GetDestructPrefix()...) // both m1 and m2
    out.IngestedSstPaths = append(out.IngestedSstPaths, m1.GetIngestedSstPaths()...)
    // ^^^ only m1, m2's IngestedSstPaths are silently dropped
    return out
}

All other repeated fields (Compactions, DestructPrefix) merge both m1 and m2. IngestedSstPaths is the only field that drops m2.

Impact

Storage leak: Every snapshot restore that writes SSTs to the log backup storage (via pitrCollector) creates an v1/ext_backups/{name}/ directory. After MergeAndMigrateTo (truncate) merges the corresponding .mgrt layer into BASE, the reference to this directory is lost. processExtFullBackup() never sees it, so the SST files are never cleaned up. Repeated restore operations accumulate orphan directories.

Currently masked by lock ordering: In the current system, this does not cause correctness issues because restore holds a read lock that blocks truncate until PiTR has consumed the IngestedSstPaths directly from the layer files (via Load()). By the time truncate runs and merges the layers, PiTR has already used the SSTs.

Becomes a correctness issue with lease-based lock expiration: If lock leases are introduced (allowing expired locks to be reclaimed), truncate could run before PiTR consumes the layer's IngestedSstPaths, causing data loss for PiTR.

Expected Behavior

MergeMigrations should preserve m2's IngestedSstPaths, consistent with how it handles Compactions and DestructPrefix:

out.IngestedSstPaths = append(out.IngestedSstPaths, m1.GetIngestedSstPaths()...)
out.IngestedSstPaths = append(out.IngestedSstPaths, m2.GetIngestedSstPaths()...)

This way, processExtFullBackup() can see all IngestedSstPaths and properly decide whether to keep or delete each ext_backups directory based on Finished status and TruncatedTo.

Related

This issue is a prerequisite for implementing lease-based lock expiration for log backup external storage locks.