-
Notifications
You must be signed in to change notification settings - Fork 9
Speed up the deletion of objects in trash #174
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
61877e5
98797e6
c69fb96
548cfe8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ import ( | |
| "fmt" | ||
| "path" | ||
| "strings" | ||
| "time" | ||
| "sync" | ||
|
|
||
| "github.com/pkg/errors" | ||
| "github.com/yezzey-gp/yproxy/config" | ||
|
|
@@ -109,33 +109,46 @@ func (dh *BasicGarbageMgr) DeleteGarbageInBucket(bucket string, msg message.Dele | |
| ylogger.Zero.Info().Str("bucket", bucket).Str("uploadId", upload).Msg("upload will be aborted") | ||
| } | ||
|
|
||
| if !msg.Confirm { //do not delete files if no confirmation flag provided | ||
| if !msg.Confirm { // Do not delete files if no confirmation flag provided | ||
| ylogger.Zero.Info().Msg("do not perform actual delete files as no confirmation flag provided") | ||
| return nil | ||
| } | ||
|
|
||
| /* Burst at 20% of vacuum rate capacity. It is pretty arbitrary at this time, | ||
| * but its not like something we need config field for... */ | ||
| * but its not like something we need config field for... | ||
| */ | ||
| limRate := config.InstanceConfig().VacuumCnf.FileChunkPerSec | ||
| limiter := rate.NewLimiter(rate.Limit(limRate), limRate/5) | ||
| ctx := context.Background() | ||
|
|
||
| var failedActionMsg, failedFilesMsg string | ||
| var operate func(file string) error | ||
| if msg.CrazyDrop { | ||
| failedActionMsg = "failed to delete some files" | ||
| failedFilesMsg = "some files were not deleted" | ||
| operate = func(file string) error { | ||
| ylogger.Zero.Info().Str("bucket", bucket).Str("path", file).Msg("simply delete") | ||
| return dh.StorageInterractor.DeleteObject(bucket, file) // Actual delete | ||
| } | ||
| } else { | ||
| failedActionMsg = "failed to move some files" | ||
| failedFilesMsg = "some files were not moved" | ||
| operate = func(file string) error { | ||
| tp := TrashPathFromRegPath(file, int(msg.Segnum)) | ||
| return dh.StorageInterractor.MoveObject(bucket, file, tp) | ||
| } | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need a unit test here to check if CrazyDrop not set actually Move Object happens |
||
| var failed []string | ||
| for retryCount := 0; len(fileList) > 0 && retryCount < 10; retryCount++ { | ||
| for _, file := range fileList { | ||
| /* Dont move too fast */ | ||
| /* Don't move too fast */ | ||
| if err := limiter.Wait(ctx); err != nil { | ||
| break | ||
| } | ||
| if msg.CrazyDrop { | ||
| ylogger.Zero.Info().Str("bucket", bucket).Str("path", file).Msg("simply delete") | ||
| err = dh.StorageInterractor.DeleteObject(bucket, file) | ||
| } else { | ||
| tp := TrashPathFromRegPath(file, int(msg.Segnum)) | ||
| err = dh.StorageInterractor.MoveObject(bucket, file, tp) | ||
| } | ||
| err = operate(file) | ||
| if err != nil { | ||
| ylogger.Zero.Warn().AnErr("err", err).Str("bucket", bucket).Str("file", file).Msg("failed to obsolete file") | ||
| ylogger.Zero.Warn().AnErr("err", err).Str("bucket", bucket).Str("file", file).Msg(failedActionMsg) | ||
| failed = append(failed, file) | ||
| } | ||
| } | ||
|
|
@@ -144,9 +157,9 @@ func (dh *BasicGarbageMgr) DeleteGarbageInBucket(bucket string, msg message.Dele | |
| } | ||
|
|
||
| if len(fileList) > 0 { | ||
| ylogger.Zero.Error().Str("bucket", bucket).Int("failed files count", len(fileList)).Msg("some files were not moved") | ||
| ylogger.Zero.Error().Str("bucket", bucket).Any("failed files", fileList).Msg("failed to move some files") | ||
| return errors.Wrap(err, "failed to move some files") | ||
| ylogger.Zero.Error().Str("bucket", bucket).Int("failed files count", len(fileList)).Msg(failedFilesMsg) | ||
| ylogger.Zero.Error().Str("bucket", bucket).Any("failed files", fileList).Msg(failedActionMsg) | ||
| return errors.Wrap(err, failedActionMsg) | ||
| } | ||
|
|
||
| for key, uploadId := range uploads { | ||
|
|
@@ -196,6 +209,60 @@ func (dh *BasicGarbageMgr) ListDelete2Files(bucket string, msg message.Delete2Me | |
| return filesToDelete, nil | ||
| } | ||
|
|
||
| func (dh *BasicGarbageMgr) garbageTrashParallel(bucket string, fileList []string) ([]string, error) { | ||
|
Vlasdislav marked this conversation as resolved.
|
||
| workerCount := dh.Cnf.TrashDeleteWorkers | ||
| if workerCount <= 0 { | ||
| workerCount = config.DefaultTrashDeleteWorkers | ||
| } | ||
| if workerCount > len(fileList) { | ||
| workerCount = len(fileList) | ||
| } | ||
| if workerCount == 0 { | ||
| return nil, nil | ||
| } | ||
|
|
||
| jobs := make(chan string) | ||
| failedCh := make(chan string, len(fileList)) | ||
| var wg sync.WaitGroup | ||
|
|
||
| for i := 0; i < workerCount; i++ { | ||
| wg.Go(func() { | ||
| for file := range jobs { | ||
| if err := dh.StorageInterractor.DeleteObject(bucket, file); err != nil { | ||
| ylogger.Zero.Warn().AnErr("err", err).Str("bucket", bucket).Str("file", file).Msg("failed to delete garbage file") | ||
| failedCh <- file | ||
| } | ||
| } | ||
| }) | ||
| } | ||
|
Vlasdislav marked this conversation as resolved.
|
||
|
|
||
| for _, file := range fileList { | ||
| jobs <- file | ||
| } | ||
| close(jobs) | ||
|
|
||
| wg.Wait() | ||
| close(failedCh) | ||
|
|
||
| failed := make([]string, 0, len(fileList)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In a case of failed load here we copy all file names 3d times, so the overall memory consumption will be quite big (we have a million of files). Maybe here we should use *object.ObjectInfo? extractObjectPaths also not needed in that case
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dont't we want to store failed tasks? Example, for retry
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, in a case of retry we still have list of items in memory - let's use them. Do no save it locally. Let's imagine we did it (save locally), what we will be doing at start? Check the modification date for each file once again (in order to be sure we won't delete something we do no want to), and checking is S3 listing. So we perform listing in S3 and we do not need a local list of files. |
||
| for file := range failedCh { | ||
| failed = append(failed, file) | ||
| } | ||
| if len(failed) > 0 { | ||
| return failed, errors.New("failed to delete some garbage files") | ||
| } | ||
|
|
||
| return nil, nil | ||
| } | ||
|
|
||
| func (dh *BasicGarbageMgr) extractObjectPaths(fileList []*object.ObjectInfo) []string { | ||
| paths := make([]string, 0, len(fileList)) | ||
| for _, file := range fileList { | ||
| paths = append(paths, file.Path) | ||
| } | ||
| return paths | ||
| } | ||
|
|
||
| func (dh *BasicGarbageMgr) DeletePrefixInBucket(bucket string, msg message.Delete2Message) error { | ||
|
Vlasdislav marked this conversation as resolved.
|
||
| fileList, err := dh.ListDelete2Files(bucket, msg) // Return the list of files to be deleted | ||
| if err != nil { | ||
|
|
@@ -222,27 +289,18 @@ func (dh *BasicGarbageMgr) DeletePrefixInBucket(bucket string, msg message.Delet | |
| ylogger.Zero.Info().Msg("prefix doesn't contain trash aborted") | ||
| return nil | ||
| } | ||
| trashRetention := time.Hour * 24 * time.Duration(dh.Cnf.TrashRetentionDays) | ||
| var failed []*object.ObjectInfo | ||
| for retryCount := 0; len(fileList) > 0 && retryCount < 10; retryCount++ { | ||
| for _, file := range fileList { | ||
| if strings.Contains(file.Path, "trash") && file.LastMod.Add(trashRetention).Unix() < time.Now().Unix() { | ||
| ylogger.Zero.Debug().Str("bucket", bucket).Str("path", file.Path).Msg("simply delete") | ||
| err = dh.StorageInterractor.DeleteObject(bucket, file.Path) // Actual deletion | ||
| } | ||
| if err != nil { | ||
| ylogger.Zero.Warn().AnErr("err", err).Str("bucket", bucket).Str("file", file.Path).Msg("failed to delete file") | ||
| failed = append(failed, file) | ||
| } | ||
| pathsToDelete := dh.extractObjectPaths(fileList) | ||
| for retryCount := 0; len(pathsToDelete) > 0 && retryCount < 10; retryCount++ { | ||
| pathsToDelete, err = dh.garbageTrashParallel(bucket, pathsToDelete) | ||
| if err == nil { | ||
|
Vlasdislav marked this conversation as resolved.
|
||
| break | ||
| } | ||
| fileList = failed | ||
| failed = make([]*object.ObjectInfo, 0) | ||
| } | ||
|
|
||
| if len(fileList) > 0 { | ||
| ylogger.Zero.Error().Str("bucket", bucket).Int("failed files count", len(fileList)).Msg("some files were not moved") | ||
| ylogger.Zero.Error().Str("bucket", bucket).Any("failed files", fileList).Msg("failed to move some files") | ||
| return errors.Wrap(err, "failed to move some files") | ||
| if len(pathsToDelete) > 0 { | ||
| ylogger.Zero.Error().Str("bucket", bucket).Int("failed files count", len(pathsToDelete)).Msg("some files were not deleted") | ||
| ylogger.Zero.Error().Str("bucket", bucket).Any("failed files", pathsToDelete).Msg("failed to delete some files") | ||
| return errors.Wrap(err, "failed to delete some files") | ||
| } | ||
|
|
||
| for key, uploadId := range uploads { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.