Update storage management from root labels to leases #1176
Merged
Conversation
This file contains 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
tonistiigi
force-pushed
the
cache-refs
branch
9 times, most recently
from
0816792 to
804f043
Compare
tonistiigi
force-pushed
the
cache-refs
branch
2 times, most recently
from
0be18c3 to
b314aa2
Compare
|
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 { |
There was a problem hiding this comment.
Is that newly created lease supposed to be used in this context?
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 <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
dmcgowan
reviewed
Oct 16, 2019
| @@ -106,19 +89,31 @@ func NewWorkerOpt(root string, snFactory SnapshotterFactory, rootless bool, proc | |||
| for k, v := range labels { | |||
| xlabels[k] = v | |||
| } | |||
| 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 { | |||
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
|
Needs rebase |
tonistiigi
force-pushed
the
cache-refs
branch
from
467f467 to
339d4b2
Compare
AkihiroSuda
approved these changes
Oct 18, 2019
This was referenced Oct 22, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.