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

Unified Repository Design #4926

Merged
merged 17 commits into from Jul 21, 2022
Merged

Unified Repository Design #4926

merged 17 commits into from Jul 21, 2022

Conversation

Lyndon-Li
Copy link
Contributor

This PR add the initial design document for Unified Repository & Kopia Integration

@github-actions github-actions bot requested a review from a-mccarthy May 19, 2022 07:37
@Lyndon-Li Lyndon-Li marked this pull request as draft May 19, 2022 07:39
@codecov-commenter
Copy link

codecov-commenter commented May 19, 2022

Codecov Report

Merging #4926 (be820e0) into main (2464fcd) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main    #4926      +/-   ##
==========================================
- Coverage   41.29%   41.27%   -0.02%     
==========================================
  Files         211      211              
  Lines       18443    18445       +2     
==========================================
- Hits         7616     7614       -2     
- Misses      10254    10257       +3     
- Partials      573      574       +1     
Impacted Files Coverage Δ
pkg/persistence/object_store.go 56.19% <50.00%> (-0.91%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 68730cb...be820e0. Read the comment docs.

@Lyndon-Li Lyndon-Li marked this pull request as ready for review May 19, 2022 07:56
@reasonerjt reasonerjt requested review from reasonerjt, ywk253100, sseago and dsu-igeek and removed request for a-mccarthy May 19, 2022 07:57
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
Lyndon-Li and others added 2 commits May 25, 2022 10:51
Lyndon-Li and others added 2 commits June 17, 2022 09:22
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
reasonerjt
reasonerjt previously approved these changes Jul 8, 2022
@kaovilai

This comment was marked as resolved.

@kaovilai
Copy link
Contributor

Please add what to do when deleting backup containing snapshot that maybe a parent snapshot for another velero backup.
https://github.com/vmware-tanzu/velero/pull/4926/files#diff-2c3160d74871a65b38c165387821fb4627b1c6d2cd02a824ccfc0e376a4e5590

Since both restic and kopia are incremental by nature, subsequent snapshot only work if parent snapshot exists.


## CR Example
Below sample files demonstrate complete CRs with all the changes mentioned above:
- BackupRepository CR: https://gist.github.com/Lyndon-Li/f38ad69dd8c4785c046cd7ed0ef2b6ed#file-backup-repository-sample-yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed an issue, that in the spec of the backup repository CR there is still resticIdentifier will that be renamed or how does kopia repo use this field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As our current discussion, the kopia path uses storageConfig (from BSL) instead of resticIdentifier/repoIdentifier, then the resticIdentifier/repoIdentifier are for Restic only. In this way, we could keep them as is since they are Restic specific.
On the other hand, if we want to make resticIdentifier/repoIdentifier generic for other purposes, we can change the name. For now, we don't have this requirement.

@Lyndon-Li
Copy link
Contributor Author

Lyndon-Li commented Jul 19, 2022

@kaovilai Don't why I cannot reply your comments in place. Then let me make the answer here.

Both Kopia and Restic don't have the aforementioned snapshot chain problem, even though they support incremental snapshot/backup. The reason is, the incremental mechanism that is used by Kopia/Restic is file level and each snapshot contains a full view of its object data though the object data itself is shared across snapshots. Below are the details:

  • Kopia/Restic checks the file's ctime, mtime, inode, etc. to decide whether a file has no change from its parent snapshots
  • If the file is not changed, Kopia/Restic creates a new entry referring to the same object that is referred by the entry in the parent snapshot
  • After that, there will be one new reference to the object
  • When the parent snapshot is deleted, only the snapshot information is deleted. The deletion of the objects is the work of prune/maintenance
  • When a prune/maintenance happens, since there is at least one reference to the object, the object is not deleted either

@kaovilai
Copy link
Contributor

Thanks @Lyndon-Li

Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
@Lyndon-Li Lyndon-Li merged commit 67d98fe into vmware-tanzu:main Jul 21, 2022
danfengliu pushed a commit to danfengliu/velero that referenced this pull request Sep 13, 2022
Yonghui joined the velero team earlier this year.

He has been leading the effort for kopia integration, and delivered the
comprehensive comparison report for kopia .vs. restic
https://docs.google.com/document/d/1BMLuRzEpYWYE-Ci_eLg8gWbjDv4DSyqj/edit

and the detailed design for using kopia as the unified repository:
vmware-tanzu#4926

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants