Skip to content

drivers: add CreateFromTemplate()#271

Merged
rhatdan merged 5 commits intocontainers:masterfrom
nalind:template-layers
Jan 18, 2019
Merged

drivers: add CreateFromTemplate()#271
rhatdan merged 5 commits intocontainers:masterfrom
nalind:template-layers

Conversation

@nalind
Copy link
Copy Markdown
Member

@nalind nalind commented Jan 17, 2019

Add a CreateFromTemplate() method to graph drivers, and use it instead of a driver-oblivious diff/put pair when we want to create a copy of an image's top layer that has the same parent and which differs from the original only in its ID maps.

This lets drivers that can quickly make an independent layer based on another layer do something smarter than we were doing with the driver-oblivious method. For some drivers, a native method is
dramatically faster.

Note that the driver needs to be able to do this while still exposing just one notional layer (i.e., one link in the chain of layers for a given container) to the higher levels of the APIs, so if the new layer
is actually a child of the template layer, that needs to remain a detail that's private to the driver.

…ayers

Add a CreateFromTemplate() method to graph drivers, and use it instead
of a driver-oblivious diff/put method when we want to create a copy of
an image's top layer that has the same parent and which differs from the
original only in its ID maps.

This lets drivers that can quickly make an independent layer based on
another layer do something smarter than we were doing with the
driver-oblivious method.  For some drivers, a native method is
dramatically faster.

Note that the driver needs to be able to do this while still exposing
just one notional layer (i.e., one link in the chain of layers for a
given container) to the higher levels of the APIs, so if the new layer
is actually a child of the template layer, that needs to remain a detail
that's private to the driver.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When removing an image, remove the image's mapped top layers before the
image's "main" top layer, in case the graph driver is hiding a
dependency between the mapped layers and the "real" one (as it's allowed
to do).

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Make layers based on template layers actually be children of the
template layer, so that we don't need to copy the diff to create them.
Upper layers should be making sure that we don't remove the template
layer before we attempt to remove the new layer.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Jan 17, 2019

LGTM,
@giuseppe @mtrmac @rhvgoyal @umohnani8 PTAL

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Jan 17, 2019

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Jan 17, 2019

time ./bin/podman run -d --uidmap 0:514000:1000 fedora sleep 1000
0aec17b1f4e7b016533cd1e84df0a651404ae5f7761ce2b0a71dd531775638b4

real	0m1.541s
user	0m0.216s
sys	0m0.614s
sh-4.4# time ./bin/podman run -d --uidmap 0:515000:1000 fedora sleep 1000
cf6312c8d8603788383f60e54f71f72fad596a931067dd9596358985cac6b20d

real	0m1.616s
user	0m0.208s
sys	0m0.612s

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Jan 17, 2019

3-4 times faster.

 time podman run -d --uidmap 0:516000:1000 fedora sleep 1000
2d0a14e7da2ca3f0c05d167899952dac3cf78a1025abddf919fa295c155d36a4

real	0m4.787s
user	0m3.428s
sys	0m1.536s

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Jan 18, 2019

Passed all tests on Buildah, still trying on podman. Should I test this on CRI-O?

@rhvgoyal
Copy link
Copy Markdown
Contributor

I am assuming that these template layers are hidden and will not show up in image listing? Is there a way to list these, or check disk space consumed by these or to clean these up. I guess initially it is best if it is hidden from user and some sort of automatic garbage collection happens.

@giuseppe
Copy link
Copy Markdown
Member

code LGTM, but I still don't get why overlay would benefit of it, is there anything happening differently now with Diff+ApplyDiff? Could it be made clearer? Even a comment here would be fine.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Jan 18, 2019

My understanding here is that in normal layers, you commit the layer and create a tar ball of it. In this layer, we just leave the layer directory as is. This layer will never be committed to an image, so it is just for use by containers with a shifted user namespace.

One issue we might need to take care of is, cleaning up these layers. As it stands now, these layers only get removed when the is removed. We could check when a container gets removed if it is the only container that uses the layer, the layer could be removed, but if a user is constantly creating and remove the same container, this would cause a cost penalty.

Another option would be to library be smart enough to figure out layers that don't have containers associated with them, and have not been used in a very long time, remove the layer (garbage collection).

@nalind
Copy link
Copy Markdown
Member Author

nalind commented Jan 18, 2019

I am assuming that these template layers are hidden and will not show up in image listing? Is there a way to list these, or check disk space consumed by these or to clean these up. I guess initially it is best if it is hidden from user and some sort of automatic garbage collection happens.

They're still tracked as mapped top layers that are associated with the image, so that when the image is removed, they're removed along with the original version of the top layer that they're based on.

code LGTM, but I still don't get why overlay would benefit of it, is there anything happening differently now with Diff+ApplyDiff? Could it be made clearer? Even a comment here would be fine.

In overlay, if we make a new layer a child of another one, we don't need to extract and reapply a diff of the original layer's contents to make them show up when we mount the child layer. When that layer contains dozens or hundreds of megabytes of contents, walking the filesystem, reading the file contents, pushing them through a pipe, and then writing them back out to disk was what was chewing up a majority of the time in the case @rhatdan was testing with.

On current systems, we still take a hit because chown() triggers a full copy up (give or take the FICLONE ioctl on filesystems like XFS with the reflink option enabled), but once the kernel overlay filesystem's "metacopy" logic lands, changes that don't affect a file's contents will switch from being handled by copying the lower file's contents to the upper layer, to creating a sparse file in the upper layer with a "trusted.overlay.metacopy" extended attribute, which we weren't able to take advantage of before.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Jan 18, 2019

By the way @nalind I think this is a brilliant solution.

@rhatdan rhatdan merged commit 8910180 into containers:master Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants