Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression in chunk deletion for openstack provider #703

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
21 changes: 17 additions & 4 deletions pkg/snapshot/snapshotter/garbagecollector.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,23 @@ func (ssr *Snapshotter) RunGarbageCollector(stopCh <-chan struct{}) {
continue
}

// chunksDeleted stores the no of chunks deleted in the current iteration of GC
var chunksDeleted int
chunksDeleted, snapList = ssr.GarbageCollectChunks(snapList)
ssr.logger.Infof("GC: Total number garbage collected chunks: %d", chunksDeleted)
// Skip chunk deletion for openstack swift provider, since the manifest object is a virtual
// representation of the object, and the actual data is stored in the segment objects, aka chunks
// Chunk deletion for this provider is handled in regular snapshot deletion
if ssr.snapstoreConfig.Provider == brtypes.SnapstoreProviderSwift {
var filteredSnapList brtypes.SnapList
for _, snap := range snapList {
if !snap.IsChunk {
filteredSnapList = append(filteredSnapList, snap)
}
}
snapList = filteredSnapList
} else {
// chunksDeleted stores the no of chunks deleted in the current iteration of GC.
var chunksDeleted int
chunksDeleted, snapList = ssr.GarbageCollectChunks(snapList)
ssr.logger.Infof("GC: Total number garbage collected chunks: %d", chunksDeleted)
}

snapStreamIndexList := getSnapStreamIndexList(snapList)

Expand Down
55 changes: 45 additions & 10 deletions pkg/snapstore/swift_snapstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,22 +330,23 @@ func (s *SwiftSnapStore) Fetch(snap brtypes.Snapshot) (io.ReadCloser, error) {
return resp.Body, resp.Err
}

// Save will write the snapshot to store
// Save will write the snapshot to store, as a DLO (dynamic large object), as described
// in https://docs.openstack.org/swift/latest/overview_large_objects.html
func (s *SwiftSnapStore) Save(snap brtypes.Snapshot, rc io.ReadCloser) error {
// Save it locally
tmpfile, err := os.CreateTemp(s.tempDir, tmpBackupFilePrefix)
tempFile, err := os.CreateTemp(s.tempDir, tmpBackupFilePrefix)
if err != nil {
rc.Close()
return fmt.Errorf("failed to create snapshot tempfile: %v", err)
}
defer func() {
tmpfile.Close()
os.Remove(tmpfile.Name())
tempFile.Close()
os.Remove(tempFile.Name())
}()
size, err := io.Copy(tmpfile, rc)
size, err := io.Copy(tempFile, rc)
rc.Close()
if err != nil {
return fmt.Errorf("failed to save snapshot to tmpfile: %v", err)
return fmt.Errorf("failed to save snapshot to tempFile: %v", err)
}

var (
Expand All @@ -365,7 +366,7 @@ func (s *SwiftSnapStore) Save(snap brtypes.Snapshot, rc io.ReadCloser) error {

for i := uint(0); i < s.maxParallelChunkUploads; i++ {
wg.Add(1)
go s.chunkUploader(&wg, cancelCh, &snap, tmpfile, chunkUploadCh, resCh)
go s.chunkUploader(&wg, cancelCh, &snap, tempFile, chunkUploadCh, resCh)
}

logrus.Infof("Uploading snapshot of size: %d, chunkSize: %d, noOfChunks: %d", size, chunkSize, noOfChunks)
Expand Down Expand Up @@ -483,13 +484,47 @@ func (s *SwiftSnapStore) List() (brtypes.SnapList, error) {

sort.Sort(snapList)
return snapList, nil
}

func (s *SwiftSnapStore) getSnapshotChunks(snapshot brtypes.Snapshot) (brtypes.SnapList, error) {
snaps, err := s.List()
if err != nil {
return nil, err
}

var chunkList brtypes.SnapList
for _, snap := range snaps {
if snap.IsChunk {
chunkParentSnapPath, _ := path.Split(path.Join(snap.Prefix, snap.SnapDir, snap.SnapName))
if strings.TrimSuffix(chunkParentSnapPath, "/") == path.Join(snapshot.Prefix, snapshot.SnapDir, snapshot.SnapName) {
chunkList = append(chunkList, snap)
}
}
}
return chunkList, nil
}

// Delete should delete the snapshot file from store
// Delete deletes the objects related to the DLO (dynamic large object) from the store.
// This includes the manifest object as well as the segment objects, as
// described in https://docs.openstack.org/swift/latest/overview_large_objects.html
func (s *SwiftSnapStore) Delete(snap brtypes.Snapshot) error {
result := objects.Delete(s.client, s.bucket, path.Join(snap.Prefix, snap.SnapDir, snap.SnapName), nil)
return result.Err
chunks, err := s.getSnapshotChunks(snap)
if err != nil {
return err
}

var chunkObjectNames []string
for _, chunk := range chunks {
chunkObjectNames = append(chunkObjectNames, path.Join(chunk.Prefix, chunk.SnapDir, chunk.SnapName))
}

manifestObjectDeleteResult := objects.Delete(s.client, s.bucket, path.Join(snap.Prefix, snap.SnapDir, snap.SnapName), nil)
if manifestObjectDeleteResult.Err != nil {
return manifestObjectDeleteResult.Err
}
shreyas-s-rao marked this conversation as resolved.
Show resolved Hide resolved
shreyas-s-rao marked this conversation as resolved.
Show resolved Hide resolved

chunkObjectsDeleteResult := objects.BulkDelete(s.client, s.bucket, chunkObjectNames)
return chunkObjectsDeleteResult.Err
shreyas-s-rao marked this conversation as resolved.
Show resolved Hide resolved
}

// GetSwiftCredentialsLastModifiedTime returns the latest modification timestamp of the Swift credential file(s)
Expand Down