Speed up the deletion of objects in trash#174
Conversation
Vlasdislav
force-pushed
the
feat/parallel-trash-removal
branch
from
66dc87b to
61877e5
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to speed up deletion of objects under the trash prefix by performing deletions in parallel, with the parallelism controlled via configuration.
Changes:
- Refactors
DeleteGarbageInBucketto unify “move to trash” vs “delete immediately” behaviors behind a shared operation function and improved logging messages. - Introduces a worker-pool based parallel deletion path for trash cleanup and adds
trash_delete_workersto vacuum configuration. - Improves inline documentation/comments for
DeleteMessagefields.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/proc/delete_handler.go | Adds parallel trash deletion helper and refactors garbage deletion flow/log messages. |
| pkg/message/delete_message.go | Adds clearer field comments for delete message semantics. |
| config/vacuum.go | Adds TrashDeleteWorkers config field for parallel deletion. |
| config/instance.go | Adds default value embedding for TrashDeleteWorkers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return dh.StorageInterractor.MoveObject(bucket, file, tp) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
we need a unit test here to check if CrazyDrop not set actually Move Object happens
| wg.Wait() | ||
| close(failedCh) | ||
|
|
||
| failed := make([]string, 0, len(fileList)) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Dont't we want to store failed tasks? Example, for retry
There was a problem hiding this comment.
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.
Delete data in parallel from trash, and the degree of parallelism is a configuration parameter