Refcount driver mounts #3073
Refcount driver mounts #3073
Conversation
| @@ -85,6 +85,9 @@ func (d *Driver) Get(id string) (string, error) { | |||
| return dir, nil | |||
| } | |||
|
|
|||
| func (d *Driver) Put(id string) { | |||
| } | |||
There was a problem hiding this comment.
Could you add a comment why we do not need to implement Put on vfs?
|
New version uses atomics and adds a comment to Vfs Put() implementation. It also removes the deprecated container.EnsureMounted() |
|
This is really needed. Not sure about the Put name (maybe "Release"?) but we should review this this week if possible. /cc @crosbymichael @creack @vieux |
| @@ -222,6 +222,9 @@ func (a *Driver) Get(id string) (string, error) { | |||
| return out, nil | |||
| } | |||
|
|
|||
| func (a *Driver) Put(id string) { | |||
There was a problem hiding this comment.
This need to be implemented for aufs as well
There was a problem hiding this comment.
Yeah, i kinda left that out as I don't regularly run the aufs backend, but its no worse of than currently without this implemented :)
|
New version fixes the comments above, (well, still no aufs support, and its still called Put). |
|
@alexlarsson Let me know if you need assistance in order to implement the AUFS Put() when you come back. |
|
Updated series with signoffs. Will take a look at aufs next. |
|
Added aufs version of Put(). Tested in vagrant and seems to work well. |
|
@alexlarsson Can you rebase, please? |
|
rebased on master |
|
@alexlarsson It seems this needs a go fmt. Can you please |
|
@alexlarsson The integration tests are failing because |
This makes all users of Put() have a corresponding call to Get() which means we will be able to track whether any particular ID is in use and if not unmount it. Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
This was deprecated already and all it did was call Mount(). The use of this was a bit confusing since we need to pair Mount/Unmount calls which wasn't obvious with "EnsureMounted". Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
This implements the new Put() operation such that Get()/Put() maintains a refcount for each ID, mounting only on first Get() and unmounting on the last Get(). This means we avoid littering the system with lots of mounts and active devicemapper devices and free resources related to them. Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson) asdfljk
This implements the new Put() operation such that Get()/Put() maintains a refcount for each ID, mounting only on first Get() and unmounting on the last Get(). This means we avoid littering the system with lots of mounts and free resources related to them. Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
|
Sorry, i missed the new EnsureMounted() call, new rebased series includes that. |
|
LGTM ping @creack @crosbymichael |
|
LGTM |
1 similar comment
|
LGTM |
This adds a Put() operation to the driver API which pairs with the Get() call.
All callers of Get() are changed to have a corresponding Put() which lets us balance the uses of a particular ID.
We then use this in the devicemapper driver to avoid leaving mounts around.
For instance, we almost never need any images of init layers mounted, only when e.g. diffing or calculating sizes. We also don't want to leave stray mounts for every container that was ever run (we currently even mount old ones on daemon start!).