Update storage management from root labels to leases#1176
Merged
AkihiroSuda merged 15 commits intomoby:masterfrom Oct 18, 2019
Merged
Update storage management from root labels to leases#1176AkihiroSuda merged 15 commits intomoby:masterfrom
AkihiroSuda merged 15 commits intomoby:masterfrom
Conversation
0816792 to
804f043
Compare
0be18c3 to
b314aa2
Compare
Member
Author
|
Finished up the migration and this should be ready now. @fuweid Can you check that the |
dmcgowan
reviewed
Oct 2, 2019
dmcgowan
reviewed
Oct 2, 2019
dmcgowan
reviewed
Oct 2, 2019
dmcgowan
reviewed
Oct 2, 2019
dmcgowan
reviewed
Oct 2, 2019
| return nil, errors.Wrapf(err, "failed to add snapshot %s to lease", id) | ||
| } | ||
|
|
||
| if err := cm.Snapshotter.Prepare(ctx, id, parentSnapshotID); err != nil { |
Member
There was a problem hiding this comment.
Is that newly created lease supposed to be used in this context?
Member
Author
There was a problem hiding this comment.
No, the id is already part of the permanent lease from the AddResource
dmcgowan
reviewed
Oct 2, 2019
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
dmcgowan
reviewed
Oct 16, 2019
| } | ||
| snap := containerdsnapshot.NewSnapshotter(snFactory.Name, mdb.Snapshotter(snFactory.Name), "buildkit", idmap) | ||
| lm := leaseutil.WithNamespace(ctdmetadata.NewLeaseManager(mdb), "buildkit") | ||
| if err := cache.MigrateV2(context.TODO(), filepath.Join(root, "metadata.db"), filepath.Join(root, "metadata_v2.db"), c, snap, lm); err != nil { |
Member
There was a problem hiding this comment.
Is this context going to be passed in? The calling function signature looks like it could use some attention
Member
|
Needs rebase |
467f467 to
339d4b2
Compare
AkihiroSuda
approved these changes
Oct 18, 2019
This was referenced Oct 22, 2019
This was referenced Feb 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update all buildkit storage management from using root labels for containerd snapshots and blobs to using leases instead.
Every reference in
buildctl duis now backed by non-expiring lease. Snapshots and blobs are added to the lease during the lifetime of the cache record. A record can have only a snapshot, only a blob or both. Records with only one can pick up another later (eg. after differ runs). These leases are also used for additional metadata blobs like image configs and manifests.Tracking of blob metadata is now moved directly into the cache package.
Using leases helps to avoid some races caused by the label management not being completely atomic. It also means that buildkit doesn't require full ownership of the containerd namespace anymore.
Another change is that because chainid-s are not used directly in pull, a chainid calculated by a differ can now be matched with the pulled blob.
This work depends on the flat-leases feature in containerd 1.3 . Without it metadata blobs can have back-references to layers, causing them to not get released. So containerd 1.2 and lower will not be supported after this PR.
TODO: migration from old storage metadata. Current PR only works from an empty state.