{"action":"created","comment":{"url":"https://api.github.com/repos/docker/docker/pulls/comments/73186904","id":73186904,"diff_hunk":"@@ -92,7 +92,9 @@ func Unregister(name string) bool {\n // driver with the given name has not been registered it checks if\n // there is a VolumeDriver plugin available with the given name.\n func lookup(name string) (volume.Driver, error) {\n-\tdrivers.driverLock.Lock(name)\n+\tif err := drivers.driverLock.Lock(name); err != nil {\n+\t\treturn nil, fmt.Errorf(\"Error looking up volume plugin %s: %v\", name, err)\n+\t}\n \tdefer drivers.driverLock.Unlock(name)","path":"volume/drivers/extpoint.go","position":8,"original_position":8,"commit_id":"b171d9e3e865a34a9b9bd681e39c0101b97e3760","original_commit_id":"b793c670eef6b901a8dc8630ad06346edaf9d6ff","user":{"login":"anusha-ragunathan","id":14184178,"avatar_url":"https://avatars.githubusercontent.com/u/14184178?v=3","gravatar_id":"","url":"https://api.github.com/users/anusha-ragunathan","html_url":"https://github.com/anusha-ragunathan","followers_url":"https://api.github.com/users/anusha-ragunathan/followers","following_url":"https://api.github.com/users/anusha-ragunathan/following{/other_user}","gists_url":"https://api.github.com/users/anusha-ragunathan/gists{/gist_id}","starred_url":"https://api.github.com/users/anusha-ragunathan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/anusha-ragunathan/subscriptions","organizations_url":"https://api.github.com/users/anusha-ragunathan/orgs","repos_url":"https://api.github.com/users/anusha-ragunathan/repos","events_url":"https://api.github.com/users/anusha-ragunathan/events{/privacy}","received_events_url":"https://api.github.com/users/anusha-ragunathan/received_events","type":"User","site_admin":false},"body":"My bad, what you have is fine. I'm used to seeing this format: \r\nerr := Lock()\r\ndefer UnLock()\r\nif err != nil {\r\n// handle error\r\n}","created_at":"2016-08-02T16:13:01Z","updated_at":"2016-08-02T16:13:01Z","html_url":"https://github.com/docker/docker/pull/25005#discussion_r73186904","pull_request_url":"https://api.github.com/repos/docker/docker/pulls/25005","_links":{"self":{"href":"https://api.github.com/repos/docker/docker/pulls/comments/73186904"},"html":{"href":"https://github.com/docker/docker/pull/25005#discussion_r73186904"},"pull_request":{"href":"https://api.github.com/repos/docker/docker/pulls/25005"}}},"pull_request":{"url":"https://api.github.com/repos/docker/docker/pulls/25005","id":78682416,"html_url":"https://github.com/docker/docker/pull/25005","diff_url":"https://github.com/docker/docker/pull/25005.diff","patch_url":"https://github.com/docker/docker/pull/25005.patch","issue_url":"https://api.github.com/repos/docker/docker/issues/25005","number":25005,"state":"open","locked":false,"title":"Fix plugin lock when restoring containers","user":{"login":"ohmystack","id":2697228,"avatar_url":"https://avatars.githubusercontent.com/u/2697228?v=3","gravatar_id":"","url":"https://api.github.com/users/ohmystack","html_url":"https://github.com/ohmystack","followers_url":"https://api.github.com/users/ohmystack/followers","following_url":"https://api.github.com/users/ohmystack/following{/other_user}","gists_url":"https://api.github.com/users/ohmystack/gists{/gist_id}","starred_url":"https://api.github.com/users/ohmystack/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ohmystack/subscriptions","organizations_url":"https://api.github.com/users/ohmystack/orgs","repos_url":"https://api.github.com/users/ohmystack/repos","events_url":"https://api.github.com/users/ohmystack/events{/privacy}","received_events_url":"https://api.github.com/users/ohmystack/received_events","type":"User","site_admin":false},"body":"The problem occurs when starting `dockerd` with the containers using the\r\nsame unavailable plugin.\r\nThe containers are restored one-by-one, not asynchronous.\r\nThe dockerd needs to wait (N)*(8 secs) for restoring all the containers.\r\n(N is the number of containers using the unavailable plugin.)\r\n\r\n## How to reproduce\r\n\r\n> Precondition: I have a `rbd-docker-plugin`, with a plugin name \"rbd\")\r\n\r\n**Step 1.** Run the `rbd-docker-plugin` and the `dockerd`.\r\n**Step 2.** Create 2 containers using plugin `rbd`.\r\n\r\n```\r\ndocker create --name cont01 --volume-driver=rbd -v docker_volumes/web1:/aaa ubuntu:14.04     sleep 5\r\ndocker create --name cont02 --volume-driver=rbd -v docker_volumes/web1:/aaa ubuntu:14.04 sleep 5\r\n```\r\n\r\n**Step 3.** Stop `rbd-docker-plugin`.\r\n**Step 4.** Restart `dockerd`. (Now dockerd is unable to connect plugin `rbd`)\r\n\r\nThen, you will find the `dockerd` logs like this:\r\n\r\n```plain\r\nWARN[0001] Unable to locate plugin: rbd, retrying in 1s\r\nWARN[0002] Unable to locate plugin: rbd, retrying in 2s\r\nWARN[0004] Unable to locate plugin: rbd, retrying in 4s\r\nWARN[0008] Unable to locate plugin: rbd, retrying in 8s\r\nERRO[0016] get docker_volumes/web1: Error looking up volume plugin rbd: plugin not found\r\nWARN[0016] Unable to locate plugin: rbd, retrying in 1s\r\nWARN[0017] Unable to locate plugin: rbd, retrying in 2s\r\nWARN[0019] Unable to locate plugin: rbd, retrying in 4s\r\nWARN[0023] Unable to locate plugin: rbd, retrying in 8s\r\nERRO[0031] get docker_volumes/web1: Error looking up volume plugin rbd: plugin not found\r\n```\r\n\r\nTotally use 2 * 16 seconds = 32 secs.\r\n\r\n## Should be\r\nEvery container, who waits for the plugin, should start to wait at the same time, like this,\r\n\r\n```plain\r\nWARN[0001] Unable to locate plugin: rbd, retrying in 1s\r\nWARN[0002] Unable to locate plugin: rbd, retrying in 2s\r\nWARN[0004] Unable to locate plugin: rbd, retrying in 4s\r\nWARN[0008] Unable to locate plugin: rbd, retrying in 8s\r\nERRO[0016] get docker_volumes/web1: Error looking up volume plugin rbd: plugin not found\r\nERRO[0016] get docker_volumes/web1: Error looking up volume plugin rbd: plugin not found\r\n```\r\n\r\nTotally use 16 seconds.\r\n\r\n\r\n## The cause\r\n~~Though [daemon.prepareMountPoints(c)](https://github.com/docker/docker/blob/v1.12.0-rc4/daemon/daemon.go#L298-L303) runs in seperated goroutines,\r\ncontainers are still initialized synchronously because it is blocked at\r\n[\"GetWithRef\"](https://github.com/docker/docker/blob/v1.12.0-rc4/volume/store/store.go#L319-L324).~~\r\n\r\nWhen multiple goroutines call [`drivers.lookup`](https://github.com/docker/docker/blob/v1.12.0-rc4/volume/drivers/extpoint.go#L91-L123) for the same volume concurrently,\r\n\r\n* There are 2 steps in `lookup` to initialize a volume driver.\r\n  * **Step1.** Get the plugin by `plugin.LookupWithCapability(name, extName)`, put plugin into `storage.plugins[name]`\r\n  * **Step2.** Put plugin into `drivers.extensions[name]`\r\n* The wait loop (waits 16s) is in **Step1**. `lookup` holds a lock (for **Step1** + **Step2**) at the beginning, which is before **Step1**'s lock. So that the 2nd goroutine's **Step1** cannot go into its wait loop, Every goroutine's **Step1** becomes synchronous.\r\n  * When the 2nd goroutine's **Step1** can run, it doesn't know this plugin with this \"name\" has failed before (in 1st goroutine)\r\n  * The 2nd goroutine waits again, waits another 16s.\r\n  * Totally, it becomes (N)*(16s).\r\n\r\n## Here's the fixes\r\n1. The 1st goroutine holds the lock\r\n2. The 2nd, 3rd, ..., goroutines wait for the lock\r\n3. If the 1st goroutine's **Step1** + **Step2** fails, it tells 2nd, 3rd, ..., other goroutines to return error, not to do **Step1** + **Step2** again.\r\n\r\n~~Only after getting the volume driver successfully should the volume be locked by name.~~\r\n\r\n~~There is no need to lock drivers when \"lookup\" a driver. Because\r\n  [\"lookup\"](https://github.com/docker/docker/blob/v1.12.0-rc4/volume/drivers/extpoint.go#L91-L123) actually does a GetOrCreate thing. When it is doing\r\n  [(pkg/plugins/plugins.go)get(name string)](https://github.com/docker/docker/blob/v1.12.0-rc4/pkg/plugins/plugins.go#L200-L202), there is already a lock to\r\n  ensure the same plugin won't be registered twice. So we can remove\r\n  the outer lock in \"lookup\". Nor, it will block the \"prepareMountPoints\".~~\r\n\r\n\r\n","created_at":"2016-07-25T13:38:34Z","updated_at":"2016-08-02T16:13:01Z","closed_at":null,"merged_at":null,"merge_commit_sha":"d4ce050f244767c5629e4680761f971927f5c954","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/docker/docker/pulls/25005/commits","review_comments_url":"https://api.github.com/repos/docker/docker/pulls/25005/comments","review_comment_url":"https://api.github.com/repos/docker/docker/pulls/comments{/number}","comments_url":"https://api.github.com/repos/docker/docker/issues/25005/comments","statuses_url":"https://api.github.com/repos/docker/docker/statuses/b171d9e3e865a34a9b9bd681e39c0101b97e3760","head":{"label":"ohmystack:fix/plugin-lock","ref":"fix/plugin-lock","sha":"b171d9e3e865a34a9b9bd681e39c0101b97e3760","user":{"login":"ohmystack","id":2697228,"avatar_url":"https://avatars.githubusercontent.com/u/2697228?v=3","gravatar_id":"","url":"https://api.github.com/users/ohmystack","html_url":"https://github.com/ohmystack","followers_url":"https://api.github.com/users/ohmystack/followers","following_url":"https://api.github.com/users/ohmystack/following{/other_user}","gists_url":"https://api.github.com/users/ohmystack/gists{/gist_id}","starred_url":"https://api.github.com/users/ohmystack/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ohmystack/subscriptions","organizations_url":"https://api.github.com/users/ohmystack/orgs","repos_url":"https://api.github.com/users/ohmystack/repos","events_url":"https://api.github.com/users/ohmystack/events{/privacy}","received_events_url":"https://api.github.com/users/ohmystack/received_events","type":"User","site_admin":false},"repo":{"id":64125716,"name":"docker","full_name":"ohmystack/docker","owner":{"login":"ohmystack","id":2697228,"avatar_url":"https://avatars.githubusercontent.com/u/2697228?v=3","gravatar_id":"","url":"https://api.github.com/users/ohmystack","html_url":"https://github.com/ohmystack","followers_url":"https://api.github.com/users/ohmystack/followers","following_url":"https://api.github.com/users/ohmystack/following{/other_user}","gists_url":"https://api.github.com/users/ohmystack/gists{/gist_id}","starred_url":"https://api.github.com/users/ohmystack/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ohmystack/subscriptions","organizations_url":"https://api.github.com/users/ohmystack/orgs","repos_url":"https://api.github.com/users/ohmystack/repos","events_url":"https://api.github.com/users/ohmystack/events{/privacy}","received_events_url":"https://api.github.com/users/ohmystack/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/ohmystack/docker","description":"Docker - the open-source application container engine","fork":true,"url":"https://api.github.com/repos/ohmystack/docker","forks_url":"https://api.github.com/repos/ohmystack/docker/forks","keys_url":"https://api.github.com/repos/ohmystack/docker/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ohmystack/docker/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ohmystack/docker/teams","hooks_url":"https://api.github.com/repos/ohmystack/docker/hooks","issue_events_url":"https://api.github.com/repos/ohmystack/docker/issues/events{/number}","events_url":"https://api.github.com/repos/ohmystack/docker/events","assignees_url":"https://api.github.com/repos/ohmystack/docker/assignees{/user}","branches_url":"https://api.github.com/repos/ohmystack/docker/branches{/branch}","tags_url":"https://api.github.com/repos/ohmystack/docker/tags","blobs_url":"https://api.github.com/repos/ohmystack/docker/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ohmystack/docker/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ohmystack/docker/git/refs{/sha}","trees_url":"https://api.github.com/repos/ohmystack/docker/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ohmystack/docker/statuses/{sha}","languages_url":"https://api.github.com/repos/ohmystack/docker/languages","stargazers_url":"https://api.github.com/repos/ohmystack/docker/stargazers","contributors_url":"https://api.github.com/repos/ohmystack/docker/contributors","subscribers_url":"https://api.github.com/repos/ohmystack/docker/subscribers","subscription_url":"https://api.github.com/repos/ohmystack/docker/subscription","commits_url":"https://api.github.com/repos/ohmystack/docker/commits{/sha}","git_commits_url":"https://api.github.com/repos/ohmystack/docker/git/commits{/sha}","comments_url":"https://api.github.com/repos/ohmystack/docker/comments{/number}","issue_comment_url":"https://api.github.com/repos/ohmystack/docker/issues/comments{/number}","contents_url":"https://api.github.com/repos/ohmystack/docker/contents/{+path}","compare_url":"https://api.github.com/repos/ohmystack/docker/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ohmystack/docker/merges","archive_url":"https://api.github.com/repos/ohmystack/docker/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ohmystack/docker/downloads","issues_url":"https://api.github.com/repos/ohmystack/docker/issues{/number}","pulls_url":"https://api.github.com/repos/ohmystack/docker/pulls{/number}","milestones_url":"https://api.github.com/repos/ohmystack/docker/milestones{/number}","notifications_url":"https://api.github.com/repos/ohmystack/docker/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ohmystack/docker/labels{/name}","releases_url":"https://api.github.com/repos/ohmystack/docker/releases{/id}","deployments_url":"https://api.github.com/repos/ohmystack/docker/deployments","created_at":"2016-07-25T10:30:55Z","updated_at":"2016-07-25T10:31:10Z","pushed_at":"2016-08-02T03:13:49Z","git_url":"git://github.com/ohmystack/docker.git","ssh_url":"git@github.com:ohmystack/docker.git","clone_url":"https://github.com/ohmystack/docker.git","svn_url":"https://github.com/ohmystack/docker","homepage":"http://www.docker.com","size":97101,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"docker:master","ref":"master","sha":"8f78dae4e308e47eb6cdba643c995a8a9b46b0cd","user":{"login":"docker","id":5429470,"avatar_url":"https://avatars.githubusercontent.com/u/5429470?v=3","gravatar_id":"","url":"https://api.github.com/users/docker","html_url":"https://github.com/docker","followers_url":"https://api.github.com/users/docker/followers","following_url":"https://api.github.com/users/docker/following{/other_user}","gists_url":"https://api.github.com/users/docker/gists{/gist_id}","starred_url":"https://api.github.com/users/docker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/docker/subscriptions","organizations_url":"https://api.github.com/users/docker/orgs","repos_url":"https://api.github.com/users/docker/repos","events_url":"https://api.github.com/users/docker/events{/privacy}","received_events_url":"https://api.github.com/users/docker/received_events","type":"Organization","site_admin":false},"repo":{"id":7691631,"name":"docker","full_name":"docker/docker","owner":{"login":"docker","id":5429470,"avatar_url":"https://avatars.githubusercontent.com/u/5429470?v=3","gravatar_id":"","url":"https://api.github.com/users/docker","html_url":"https://github.com/docker","followers_url":"https://api.github.com/users/docker/followers","following_url":"https://api.github.com/users/docker/following{/other_user}","gists_url":"https://api.github.com/users/docker/gists{/gist_id}","starred_url":"https://api.github.com/users/docker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/docker/subscriptions","organizations_url":"https://api.github.com/users/docker/orgs","repos_url":"https://api.github.com/users/docker/repos","events_url":"https://api.github.com/users/docker/events{/privacy}","received_events_url":"https://api.github.com/users/docker/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/docker/docker","description":"Docker - the open-source application container engine","fork":false,"url":"https://api.github.com/repos/docker/docker","forks_url":"https://api.github.com/repos/docker/docker/forks","keys_url":"https://api.github.com/repos/docker/docker/keys{/key_id}","collaborators_url":"https://api.github.com/repos/docker/docker/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/docker/docker/teams","hooks_url":"https://api.github.com/repos/docker/docker/hooks","issue_events_url":"https://api.github.com/repos/docker/docker/issues/events{/number}","events_url":"https://api.github.com/repos/docker/docker/events","assignees_url":"https://api.github.com/repos/docker/docker/assignees{/user}","branches_url":"https://api.github.com/repos/docker/docker/branches{/branch}","tags_url":"https://api.github.com/repos/docker/docker/tags","blobs_url":"https://api.github.com/repos/docker/docker/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/docker/docker/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/docker/docker/git/refs{/sha}","trees_url":"https://api.github.com/repos/docker/docker/git/trees{/sha}","statuses_url":"https://api.github.com/repos/docker/docker/statuses/{sha}","languages_url":"https://api.github.com/repos/docker/docker/languages","stargazers_url":"https://api.github.com/repos/docker/docker/stargazers","contributors_url":"https://api.github.com/repos/docker/docker/contributors","subscribers_url":"https://api.github.com/repos/docker/docker/subscribers","subscription_url":"https://api.github.com/repos/docker/docker/subscription","commits_url":"https://api.github.com/repos/docker/docker/commits{/sha}","git_commits_url":"https://api.github.com/repos/docker/docker/git/commits{/sha}","comments_url":"https://api.github.com/repos/docker/docker/comments{/number}","issue_comment_url":"https://api.github.com/repos/docker/docker/issues/comments{/number}","contents_url":"https://api.github.com/repos/docker/docker/contents/{+path}","compare_url":"https://api.github.com/repos/docker/docker/compare/{base}...{head}","merges_url":"https://api.github.com/repos/docker/docker/merges","archive_url":"https://api.github.com/repos/docker/docker/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/docker/docker/downloads","issues_url":"https://api.github.com/repos/docker/docker/issues{/number}","pulls_url":"https://api.github.com/repos/docker/docker/pulls{/number}","milestones_url":"https://api.github.com/repos/docker/docker/milestones{/number}","notifications_url":"https://api.github.com/repos/docker/docker/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/docker/docker/labels{/name}","releases_url":"https://api.github.com/repos/docker/docker/releases{/id}","deployments_url":"https://api.github.com/repos/docker/docker/deployments","created_at":"2013-01-18T18:10:57Z","updated_at":"2016-08-02T16:01:00Z","pushed_at":"2016-08-02T15:58:40Z","git_url":"git://github.com/docker/docker.git","ssh_url":"git@github.com:docker/docker.git","clone_url":"https://github.com/docker/docker.git","svn_url":"https://github.com/docker/docker","homepage":"http://www.docker.com","size":204549,"stargazers_count":33663,"watchers_count":33663,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":9792,"mirror_url":null,"open_issues_count":2090,"forks":9792,"open_issues":2090,"watchers":33663,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/docker/docker/pulls/25005"},"html":{"href":"https://github.com/docker/docker/pull/25005"},"issue":{"href":"https://api.github.com/repos/docker/docker/issues/25005"},"comments":{"href":"https://api.github.com/repos/docker/docker/issues/25005/comments"},"review_comments":{"href":"https://api.github.com/repos/docker/docker/pulls/25005/comments"},"review_comment":{"href":"https://api.github.com/repos/docker/docker/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/docker/docker/pulls/25005/commits"},"statuses":{"href":"https://api.github.com/repos/docker/docker/statuses/b171d9e3e865a34a9b9bd681e39c0101b97e3760"}}}}
{"actor":{"display_login":"anusha-ragunathan"}}