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

Enable docker to choose a different graphdriver for volume #11354

Closed
wants to merge 1 commit into from

Conversation

yasker
Copy link
Contributor

@yasker yasker commented Mar 12, 2015

The volume driver other than VFS would create internal directory under /var/docker/volumes, rather than /var/docker, to avoid conflict with graphdriver for images

This patch has been tested briefly using device mapper graphdriver. There is no migration path from other volume drivers(vfs) yet.

I don't think one volume driver per docker daemon is prefect, but that's what we have currently in docker, so this patch would live with for now. I think it's good for each volume should able to have an separate driver in some way, which would provide us much bigger flexibility on deployment.

Some related discussion happened on #8484 (comment)

@GordonTheTurtle
Copy link

Can you please sign your commits following these rules:

https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work

The easiest way to do this is to amend the last commit:

$ git clone -b "volumedriver" git@github.com:yasker/docker.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

This will update the existing PR, so you do not need to open a new one.

The volume driver other than VFS would create internal directory under
/var/docker/volumes, rather than /var/docker, to avoid conflict with graphdriver
for rootfs.

Signed-off-by: Sheng Yang <sheng.yang@rancher.com>
@cpuguy83
Copy link
Member

NOTLGTM The basic gist of this can be done today with binds and what we really want is to be able to have a mix of volumes, I'd rather not introduce this stop-gap.

@yasker
Copy link
Contributor Author

yasker commented Mar 13, 2015

At least this patch would enable the people to try and explore the usage of graphdriver during the transition time. I understand we would have per volume driver later, but in the meantime, I think enabled the possibility to use other graphdriver is a good thing, and would encourage people to involve more into the discussion that how volume driver should looks like in the future once they found graphdriver like device mapper is workable with volume.

And I am not sure how long it would take to introduce the volume driver. I think probably we can give this a try for now.

@crosbymichael
Copy link
Contributor

Actually the use of VFS here is only because it does not use a CoW filesystem and it's just used for directory management. You actually don't want to use a CoW filesystem for storing volumes because of performance for reads and writes. It will be slower and you don't want your database sitting on top of a CoW filesystem.

Also using a CoW like devicemapper or btrfs kinda creates a box for your data which is hard to get out and inspect. I would not feel comfortable promoting the use of CoW for volume(persistent data that I care about).

@rhvgoyal
Copy link
Contributor

@crosbymichael

What's wrong with using COW based volumes? I don't understand it. We are ready to use container roots and images as COW based but not volumes. That's strange. I think it is up to the user to decide what volume can be. If user wants a volume to be backed by a COW based mechanism, that's perfectly fine. It should not be docker limiting that choice.

Reads of COW based thing should not be a problem as we are reading existing. Copy. Only initial write will initiate a copy and might be slow. But again, it is up to the use case of user to decide whether they want to play that initial penalty or not. Otherwise they can create volume in a different way.

Also why it is hard to to get out and inspect data on COW based systems?

I think this conclusion that cow based volumes are bad, needs to be justified. COW based devices/files are enterprise class features to store data. And for some reason these enterprise class features are not suitable for docker volumes, that's little strange.

@rhvgoyal
Copy link
Contributor

Using volume for database is just one example. I am looking into a use case where I just want to be able to mount image/container root into another container and do the inspection. This primarily requires reading and COW based backend is great for this purpose. Create a quick snapshot of image/container root and mount it in a different container.

Given that we already have the notion of volumes, I think it makes sense to take snapshots of images or containers which show up as volume and now these volumes can be mounted inside other containers.

And for this use case, cow based volumes are perfect. So we should not limit this choice at docker level. It is up to the user based on use case whether they want a cow based volume or not.

@rhvgoyal
Copy link
Contributor

@yasker

If I am using devicemapper as storage driver and I want to use same for volume driver, how do I do that with your patch. Basically I want to use same pool to have images, container roots and volumes.

@rhvgoyal
Copy link
Contributor

@cpuguy83

If one wants to have volumes in same pool as images and container root (so that one can easily take snapshots), then bind mount solution will not work.

Supporting multiple volume drivers is just fine. But that should not exclude the case of being able to use storage driver as volume driver.

I am sort of little surprised that why did not we start from that. We already had the capability to create
images and containers from graphdriver and creating volumes also is such a natural extension to that logic. And option to use VFS as volume driver could be specified using command line option.

But we instead did it other way where we assumed that user would never want to use overlay, aufs, btrfs or devicemapper as volume driver. So let us hardcode it to VFS.

@yasker
Copy link
Contributor Author

yasker commented Mar 20, 2015

@rhvgoyal

Currently this patch separate the pool used by image and volume, because I encountered some errors when try to use the same pool. I guess it because I was using a new DeviceSet but with the same underlaying pool. Change base directory for volume solve the issue, but I can give it try in another way as well, say using the same driver instance for volume.

If I got some time next week, I probably can make a patch using the same pool for image and volume.

@rhvgoyal
Copy link
Contributor

@yasker

I don't think you can create another instance of same graphdriver to manage same pool. And that explains the reason that you could not use same pool.

And that's why I think that default volumedriver option should have been user specified storage driver. And we should allowed user to override that with a volumedriver option to VFS or to something else.

@cpuguy83
Copy link
Member

@rhvgoyal I'm working on a volume driver interface, which will possibly be a subset of the graphdriver interface...but if not it can at least be wrapped. We can certainly do something like using the already instantiated graphdriver if someone requested a volume with that driver.

@rhvgoyal
Copy link
Contributor

@cpuguy83

Nice.

So have you already put some code out somewhere which I can play with a bit and see how it is shaping up.

As long as this new interface provides me the option to create/manage volumes from existing stroage driver managing images and containers, that should help meet my needs.

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.

6 participants