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

feat: VFolder trash bin #835

Merged
merged 68 commits into from
Oct 4, 2023
Merged

feat: VFolder trash bin #835

merged 68 commits into from
Oct 4, 2023

Conversation

fregataa
Copy link
Member

@fregataa fregataa commented Oct 24, 2022

related issue: #767

  • This pr does not fully cover the related issue because this does not include auto-clearing trash bin.

1. impl trash bin in vfs storage

  1. Now, vfolder delete API only updates the status column of vfolder table at manager, and purge API deletes the real data.
  2. add vfolder statuses such as DELETE_ONGOING or PURGE_ONGOING etc.
  3. add vfolder status history column
    3. impl storage trash bin checker in manager dispatcher.
    storage_check_interval_days in manager's shared_config set the interval of trash bin checker.

Q: How should we determine duplicate new created vfolder name if we do not delete actual data in DB?
purge_vfolder remove the db row and remove the actual vfolder.
The name of DELETED vfolders can not be used for new created vfolders but the name of PURGED vfolders are available.
-> Decided to remove PURGED vfolder row.

@fregataa fregataa added type:feature Add new features comp:manager Related to Manager component comp:storage-proxy Related to Storage proxy component labels Oct 24, 2022
@fregataa fregataa added this to the 22.09 milestone Oct 24, 2022
@fregataa fregataa self-assigned this Oct 24, 2022
@fregataa fregataa marked this pull request as draft October 24, 2022 07:42
@github-actions github-actions bot added the comp:client Related to Client component label Oct 24, 2022
@fregataa fregataa force-pushed the feature/vfolder-trash-bin branch from 6131aa3 to dc8d30c Compare October 24, 2022 09:05
@fregataa fregataa requested a review from kyujin-cho October 24, 2022 09:05
@fregataa fregataa force-pushed the feature/vfolder-trash-bin branch from dc8d30c to 4fd6dd7 Compare October 25, 2022 07:40
@fregataa fregataa marked this pull request as ready for review October 25, 2022 07:43
Copy link
Member

@kyujin-cho kyujin-cho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need retention period feature for trash bin. Manager creates a timer for a function which looks for vFolders staying more than designated period at the trash bin and purging them automatically.

@achimnol achimnol added the urgency:blocker IT SHOULD BE RESOLVED BEFORE NEXT RELEASE! label Nov 9, 2022
@fregataa fregataa requested a review from kyujin-cho November 15, 2022 10:11
@fregataa fregataa marked this pull request as draft November 17, 2022 09:06
@fregataa
Copy link
Member Author

TODO

  1. Should we include the trash-bin in vfolder metrics?
  2. Should users have personal/group-owned trash bin? Because host-unique trash bin makes it hard to determine whose trash it is

@fregataa fregataa modified the milestones: 23.09, 24.03 Oct 4, 2023
@@ -72,6 +72,7 @@ class LockID(enum.IntEnum):
LOCKID_SCALE_TIMER = 193
LOCKID_LOG_CLEANUP_TIMER = 195
LOCKID_IDLE_CHECK_TIMER = 196
LOCKID_STORAGE_TIMER = 197
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this LockID is not used anymore.

DELETE_ONGOING = "delete-ongoing" # vfolder is being deleted
DELETE_COMPLETE = "deleted-complete" # vfolder is deleted
PURGE_ONGOING = "purge-ongoing" # vfolder is being removed permanently
# PURGE_COMPLETE = "purged-complete" # vfolder is removed permanently
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove commented out, unused snippet from the file.

VFolderOperationStatus.MOUNTED,
}
case VFolderAccessStatus.UPDATABLE:
# if UPDATABLE access status is requested, READY and MOUNTED operation statuses are accepted.
# if UPDATABLE access status is requested, only READY operation status is accepted.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this comment contradicts with actual implementation.

@fregataa fregataa requested a review from kyujin-cho October 4, 2023 04:22
@kyujin-cho kyujin-cho changed the title feat: Implement recovery of whole vfolders via trash bin feat: VFolder trash bin Oct 4, 2023
@kyujin-cho kyujin-cho enabled auto-merge October 4, 2023 05:48
@kyujin-cho kyujin-cho added this pull request to the merge queue Oct 4, 2023
Merged via the queue into main with commit b3417cc Oct 4, 2023
@kyujin-cho kyujin-cho deleted the feature/vfolder-trash-bin branch October 4, 2023 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:client Related to Client component comp:manager Related to Manager component comp:storage-proxy Related to Storage proxy component require:db-migration Automatically set when alembic migrations are added or updated size:XL 500~ LoC type:feature Add new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants