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

Graph deletes are non-atomic, db refs deleted without deleting on-disk entities #6354

Closed
ewindisch opened this issue Jun 11, 2014 · 38 comments

Comments

Projects
None yet
@ewindisch
Copy link
Contributor

commented Jun 11, 2014

In function graph.Delete,

func (graph *Graph) Delete(name string) error

The method graph.idIndex.Delete(id) is called before graph.driver.Delete(id)

It is non-atomic. If graph.driver.Delete(id) fails, the graph.idIndex.Delete has already occurred and the image is now apparently gone, but the extents may still exist on disk in whole or in part.

@LK4D4

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2014

It seems that it needs good old mutex :)

@ewindisch

This comment has been minimized.

Copy link
Contributor Author

commented Jul 7, 2014

The right solution, I believe, is support for a transaction log in the graph driver. Other databases and filesystems solve this with transaction logs and our graph driver is no different. I am prepared to begin working on this.

Pinging maintainers: @shykes @vieux @crosbymichael

@shykes

This comment has been minimized.

Copy link
Collaborator

commented Jul 10, 2014

Wouldn't it be easier to occasionally scan the graph for unreferenced dirs and just remove them? That was the thinking when we implemented this (ie "worst case we leave an unremoved dir after a crash. we can garbage collect it later").

@ewindisch

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2014

@shykes Fair point. You're suggesting we 'fsck' rather than having a filesystem journal. History shows where we've gone with that as an industry, but we also have far fewer updates than typical filesystems. I'd accept the argument that we should have both, but that 'fsck' gives us the best bang-for-our-time at the moment.

Also, I'll note that this can happen on create as well, although I haven't as throughly investigated how hard that is to reproduce and what the impacts would be.

@shykes

This comment has been minimized.

Copy link
Collaborator

commented Jul 10, 2014

Yeah that's my argument - fsck as a pragmatic stopgap, and harden it later when it makes sense to make the invest. To my knowledge we haven't ever received a real-world issue that we could track back to this, so that's a datapoint :)

Re: create, we should double-check, but from memory it's the same thing in reverse. Worst case, we crash while creating a layer (which includes populating its content from a pull), and before referencing it. The result is the same.

@ewindisch

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2014

It seems all Docker systems I run eventually run out of disk space and
simply deleting all images is not enough to return the entirety of the used
space. I've certainly seen unreferenced layers stored in /var/lib/docker.

This problem was crushing for me when I attempted to run Docker in/for CI.
In that case, I'll note that I was frequently crashing my hosts.

I have also on occasion had layers that could not be, under any
circumstances, be deleted. Those were likely a result of the create-version
of this bug.
On Jul 10, 2014 6:39 PM, "Solomon Hykes" notifications@github.com wrote:

Yeah that's my argument - fsck as a pragmatic stopgap, and harden it later
when it makes sense to make the invest. To my knowledge we haven't ever
received a real-world issue that we could track back to this, so that's a
datapoint :)

Re: create, we should double-check, but from memory it's the same thing in
reverse. Worst case, we crash while creating a layer (which includes
populating its content from a pull), and before referencing it. The result
is the same.


Reply to this email directly or view it on GitHub
#6354 (comment).

@ixti

This comment has been minimized.

Copy link

commented Jul 21, 2014

Mentioned same yesterday. When suddenly after an hour of experiments with building a test environment (for some of my stuff) found that I ran out of disk space.... /var/lib/docker/vfs was full while i had neither containers nor images (removed them all with rm and rmi).

@y3ddet

This comment has been minimized.

Copy link

commented Oct 22, 2014

This issue is still present, and occurs on multiple host configurations including Ubuntu 14.04 with LVM, UBuntu 14.04 w/ ext4, boot2docker default ext4, etc. Cleaning up the leftover files requires parsing the list of valid containers and then manually invoking "/bin/rm -rf" on the stale directories. Is this bug going to be addressed in any planned work queue?

@adamhadani

This comment has been minimized.

Copy link

commented Oct 27, 2014

@y3ddet could you share how exactly you go about recognizing stale / zombie layers and removing those? based on output of 'docker ps' / 'docker inspect' or so? Thinking of coming up with a watchdog script on cron for now until this issue is resolved

@y3ddet

This comment has been minimized.

Copy link

commented Oct 28, 2014

@adamhadani I have been able to locate the 'dangling' contents directories with the following Ruby script (depends on [https://github.com/swipely/docker-api] which is available as Ruby Gem 'docker-api'):

#!/usr/bin/ruby2.0

require 'fileutils';
require 'docker';

valid_vfsnames = {}
Docker::Container.all(:all=>true).each do |c|
  c.json['Volumes'].each do | volume,realpath | 
    if realpath.include? "/var/lib/docker/vfs/dir"
      entry = realpath.match(/[\w\d]*$/)[0]
      valid_vfsnames[entry] ||= "exists"
    end
  end
end

Dir.foreach("/var/lib/docker/vfs/dir") do | e |
  if !e.match(/\./)
    if !valid_vfsnames.keys.any? { |valid| e == valid } 
      puts "/var/lib/docker/vfs/dir/#{e} is DANGLING!"
    end
  end
end
@adamhadani

This comment has been minimized.

Copy link

commented Oct 28, 2014

@y3ddet got it, thanks. I created a similar Python version, in case becomes useful for anyone, achieves pretty much same results, for anyone who cannot introduce ruby dependencies to his environment:

#!/usr/bin/env python
"""
Check all existing Docker containers for their mapped paths, and then purge any
zombie directories in docker's volumes directory which don't correspond to an
existing container.

"""
import logging
import os
import sys
from shutil import rmtree

import docker


DOCKER_VOLUMES_DIR = "/var/lib/docker/vfs/dir"


def get_immediate_subdirectories(a_dir):
    return [os.path.join(a_dir, name) for name in os.listdir(a_dir)
            if os.path.isdir(os.path.join(a_dir, name))]


def main():
    logging.basicConfig(level=logging.INFO)

    client = docker.Client()

    valid_dirs = []
    for container in client.containers(all=True):
        volumes = client.inspect_container(container['Id'])['Volumes']
        if not volumes:
            continue

        for _, real_path in volumes.iteritems():
            if real_path.startswith(DOCKER_VOLUMES_DIR):
                valid_dirs.append(real_path)

    all_dirs = get_immediate_subdirectories(DOCKER_VOLUMES_DIR)
    invalid_dirs = set(all_dirs).difference(valid_dirs)

    logging.info("Purging %s dangling Docker volumes out of %s total volumes found.",
                 len(invalid_dirs), len(all_dirs))
    for invalid_dir in invalid_dirs:
        logging.info("Purging directory: %s", invalid_dir)
        rmtree(invalid_dir)

    logging.info("All done.")


if __name__ == "__main__":
    sys.exit(main())
@ewindisch

This comment has been minimized.

Copy link
Contributor Author

commented Oct 28, 2014

@crosbymichael - thoughts on adding a fsck util similar to the above to contrib? At least until we fix this in Docker proper with a transactional graph db?

@thaJeztah

This comment has been minimized.

Copy link
Member

commented Oct 28, 2014

For volumes, there's this as well; https://github.com/cpuguy83/docker-volumes and of course this PR; #8484

@adamhadani

This comment has been minimized.

Copy link

commented Oct 28, 2014

@thaJeztah thanks, that PR looks pretty great, could be very easily used in some one-liner / cron for getting rid of dangling dirs among other things. any idea whats the status of that as far as inclusion in a future release?

@thaJeztah

This comment has been minimized.

Copy link
Member

commented Oct 28, 2014

@adamhadani I have absolutely no idea, I hope soon! but in the mean time, the docker-volumes tool I linked (which is by the same person that created the PR) is working fine for me.

@adamhadani

This comment has been minimized.

Copy link

commented Oct 29, 2014

@thaJeztah got it, thanks again

@kumarharsh

This comment has been minimized.

Copy link

commented Nov 3, 2014

@adamhadani Thanks for the python script. Hoping a native solution to this problem lands soon... I'm frequently running out of space these days on my dev environment (which happens to be my laptop) :(

darthbinamira added a commit to darthbinamira/dotfiles that referenced this issue Nov 30, 2014

@potto007

This comment has been minimized.

Copy link

commented Dec 19, 2014

@y3ddet Thanks for posting a solution. @adamhadani Thanks for the Python script. This rescued me from a very bad situation.

@vandekeiser

This comment has been minimized.

Copy link

commented Jan 20, 2015

Still isn't fixed.. (1.4.1 in Ubuntu 14.04)
docker rmi -f $(docker images -aq) says "Error: failed to remove one or more images"
But then i can see that disk space use doesn't go back to baseline, and /var/lib/docker/vfs/dir/ is not empty
If i rm /var/lib/docker/vfs/dir/, then df (almost) goes back to baseline

@FelikZ

This comment has been minimized.

Copy link

commented Feb 1, 2015

@adamhadani could you post this script to github gists? It still an issue and while it is, this script is extremely useful

@adamhadani

This comment has been minimized.

Copy link

commented Feb 5, 2015

@soupdiver

This comment has been minimized.

Copy link

commented Feb 12, 2015

I just came across the exact same problem and then discovered that there is the option docker rm -v. I had in my memory that volumes will be deleted automatically when no container has a reference anymore to it. Does the problem discussed here just related to the auto deletion of volumes or also to the explicit deletion?

@cpuguy83

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2015

@soupdiver There is no auto deletion of volumes at all. The only time a volume is deleted is if you explicitly docker rm -v.

@soupdiver

This comment has been minimized.

Copy link

commented Feb 12, 2015

@cpuguy83 Ah okay ... I ever thought this part from the docs Volumes persist until no containers use them means that they will be deleted after the last container referencing to a volume is gone

@thaJeztah

This comment has been minimized.

Copy link
Member

commented Feb 12, 2015

@soupdiver if that's how it's written in the docs, I agree that's confusing. Can you add a link where in the docs you found that sentence? (Then I'll create a small PR to improve that)

@soupdiver

This comment has been minimized.

Copy link

commented Feb 12, 2015

@thaJeztah It can be found in the user guide. https://docs.docker.com/userguide/dockervolumes/
Last point under Data volumes

@thaJeztah

This comment has been minimized.

Copy link
Member

commented Feb 12, 2015

@soupdiver thanks for pointing that out, I'll see if I can come up with a better description there!

@soupdiver

This comment has been minimized.

Copy link

commented Feb 12, 2015

@thaJeztah Just for clarification: Is the current behaviour that volume directories are not deleted the normal behaviour since ever? I ever thought that they will be deleted after all references are gone and I barely remember that the docs explicitly said this when I started using Docker a time ago. But I'm not 100% sure about this

thaJeztah added a commit to thaJeztah/docker that referenced this issue Feb 13, 2015

Docs: Improve description on volume removal
A comment in moby#6354 (comment)
brought to light that the "Managing Data in containers" section contained an
incorrect (or confusing) line;

  "Volumes persist until no containers use them"

Which implies that volumes are automatically removed if they are no longer
referenced by a container.

This pull-request attempts to add some information explaining that volumes are
never automatically removed by Docker and adds some extra hints on working
with data volumes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah

This comment has been minimized.

Copy link
Member

commented Feb 13, 2015

@soupdiver yes, this is normal behavior and "by design". Docker should never remove your data unless you explicitly tell it to (data is important). There are some difficulties with this (i.e., "orphaned" volumes), which is being worked on in #8484

I just created a PR that (hopefully) makes this a bit more clear, you can find it here: #10757

@soupdiver

This comment has been minimized.

Copy link

commented Feb 13, 2015

@thaJeztah ok thanks! The PR really explains the behaviour more clearly

thaJeztah added a commit to thaJeztah/docker that referenced this issue Feb 13, 2015

Docs: Improve description on volume removal
A comment in moby#6354 (comment)
brought to light that the "Managing Data in containers" section contained an
incorrect (or confusing) line;

  "Volumes persist until no containers use them"

Which implies that volumes are automatically removed if they are no longer
referenced by a container.

This pull-request attempts to add some information explaining that volumes are
never automatically removed by Docker and adds some extra hints on working
with data volumes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah added a commit to thaJeztah/docker that referenced this issue Feb 13, 2015

Docs: Improve description on volume removal
A comment in moby#6354 (comment)
brought to light that the "Managing Data in containers" section contained an
incorrect (or confusing) line;

  "Volumes persist until no containers use them"

Which implies that volumes are automatically removed if they are no longer
referenced by a container.

This pull-request attempts to add some information explaining that volumes are
never automatically removed by Docker and adds some extra hints on working
with data volumes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah added a commit to thaJeztah/docker that referenced this issue Feb 14, 2015

Docs: Improve description on volume removal
A comment in moby#6354 (comment)
brought to light that the "Managing Data in containers" section contained an
incorrect (or confusing) line;

  "Volumes persist until no containers use them"

Which implies that volumes are automatically removed if they are no longer
referenced by a container.

This pull-request attempts to add some information explaining that volumes are
never automatically removed by Docker and adds some extra hints on working
with data volumes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah added a commit to thaJeztah/docker that referenced this issue Feb 14, 2015

Docs: Improve description on volume removal
A comment in moby#6354 (comment)
brought to light that the "Managing Data in containers" section contained an
incorrect (or confusing) line;

  "Volumes persist until no containers use them"

Which implies that volumes are automatically removed if they are no longer
referenced by a container.

This pull-request attempts to add some information explaining that volumes are
never automatically removed by Docker and adds some extra hints on working
with data volumes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

brahmaroutu added a commit to brahmaroutu/docker that referenced this issue Feb 19, 2015

Docs: Improve description on volume removal
A comment in moby#6354 (comment)
brought to light that the "Managing Data in containers" section contained an
incorrect (or confusing) line;

  "Volumes persist until no containers use them"

Which implies that volumes are automatically removed if they are no longer
referenced by a container.

This pull-request attempts to add some information explaining that volumes are
never automatically removed by Docker and adds some extra hints on working
with data volumes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

spf13 added a commit to spf13/docker that referenced this issue Mar 14, 2015

Docs: Improve description on volume removal
A comment in moby#6354 (comment)
brought to light that the "Managing Data in containers" section contained an
incorrect (or confusing) line;

  "Volumes persist until no containers use them"

Which implies that volumes are automatically removed if they are no longer
referenced by a container.

This pull-request attempts to add some information explaining that volumes are
never automatically removed by Docker and adds some extra hints on working
with data volumes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@dnephin

This comment has been minimized.

Copy link
Member

commented Jun 24, 2015

We've run into this problem on our jenkins ci server as well (docker 1.5). I think the discussion about volumes is actually a tangent from the original issue. The issue is not really about volumes, it's about image layers being left on-disk after a docker rmi.

For now we're able to identify and reclaim this lost space using this bash:

#!/bin/bash

set -eu

cd /var/lib/docker/aufs/diff

for image in $(du -s * 2> /dev/null | sort -nr | grep -v -- '-init' | cut -f 2); do
    echo "Inspecting or removing $image"
    docker inspect $image > /dev/null || rm -r $image;
done
@dnephin

This comment has been minimized.

Copy link
Member

commented Sep 1, 2015

+kind/bug
+system/storage (I think)

@dalanlan

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2015

/cc @dalanlan

@vitalyisaev2

This comment has been minimized.

Copy link

commented Sep 27, 2015

@adamhadani very nice job, thank you

@presidento

This comment has been minimized.

Copy link

commented Mar 1, 2016

@dnephin We had the same issue.

But be careful: in Docker 1.10, these file names does not match with containers or images, so your script will remove everything from the diff folder. I could purge everything from every docker folder manually, but I don't know, how can we do this cleanup using docker >= 1.10.

@xiaods

This comment has been minimized.

Copy link
Contributor

commented Sep 21, 2016

this bug is outdate? anyone can handle it.

@LK4D4

This comment has been minimized.

Copy link
Contributor

commented Sep 21, 2016

I think it's duplicate of another bug from @cpuguy83

@vdemeester

This comment has been minimized.

Copy link
Member

commented Feb 14, 2018

Given the activity level on this issue, I'm going to close it as it's either fixed, a duplicate or not a request anymore. If you think I'm mistaken, feel free to discuss it there 😉

@vdemeester vdemeester closed this Feb 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.