{"action":"created","comment":{"url":"https://api.github.com/repos/jenkinsci/jenkins/pulls/comments/27131567","id":27131567,"diff_hunk":"@@ -257,21 +265,13 @@ public synchronized long check(final SlaveComputer c) {\n             } else if (c.isIdle()) {\n                 final long idleMilliseconds = System.currentTimeMillis() - c.getIdleStartMilliseconds();\n                 if (idleMilliseconds > idleDelay * 1000 * 60 /*MINS->MILLIS*/) {\n-                    Queue.withLock(new Runnable() {\n-                        @Override\n-                        public void run() {\n-                            // re-check idle now that we are within the Queue lock\n-                            if (c.isIdle()) {\n-                                final long idleMilliseconds = System.currentTimeMillis() - c.getIdleStartMilliseconds();\n-                                if (idleMilliseconds > idleDelay * 1000 * 60 /*MINS->MILLIS*/) {\n-                                    // we've been idle for long enough\n-                                    logger.log(Level.INFO, \"Disconnecting computer {0} as it has been idle for {1}\",\n-                                            new Object[]{c.getName(), Util.getTimeSpanString(idleMilliseconds)});\n-                                    c.disconnect(OfflineCause.create(Messages._RetentionStrategy_Demand_OfflineIdle()));\n-                                }\n-                            }\n-                        }\n-                    });\n+                    // we've been idle for long enough\n+                    logger.log(Level.INFO, \"Disconnecting computer {0} as it has been idle for {1}\",\n+                            new Object[]{c.getName(), Util.getTimeSpanString(idleMilliseconds)});\n+                    c.disconnect(OfflineCause.create(Messages._RetentionStrategy_Demand_OfflineIdle()));","path":"core/src/main/java/hudson/slaves/RetentionStrategy.java","position":55,"original_position":57,"commit_id":"89835c85acff5832631bb1dfd8e4003a8d444c02","original_commit_id":"0150209edecdd3be6a0d451b6d42c7273ffff763","user":{"login":"jtnord","id":494726,"avatar_url":"https://avatars.githubusercontent.com/u/494726?v=3","gravatar_id":"","url":"https://api.github.com/users/jtnord","html_url":"https://github.com/jtnord","followers_url":"https://api.github.com/users/jtnord/followers","following_url":"https://api.github.com/users/jtnord/following{/other_user}","gists_url":"https://api.github.com/users/jtnord/gists{/gist_id}","starred_url":"https://api.github.com/users/jtnord/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jtnord/subscriptions","organizations_url":"https://api.github.com/users/jtnord/orgs","repos_url":"https://api.github.com/users/jtnord/repos","events_url":"https://api.github.com/users/jtnord/events{/privacy}","received_events_url":"https://api.github.com/users/jtnord/received_events","type":"User","site_admin":false},"body":"is this correct - there is no lock held hear and I thought the check worked on a task, so it could have had something just allocated to it couldn't it?","created_at":"2015-03-25T15:27:50Z","updated_at":"2015-03-25T15:27:50Z","html_url":"https://github.com/jenkinsci/jenkins/pull/1596#discussion_r27131567","pull_request_url":"https://api.github.com/repos/jenkinsci/jenkins/pulls/1596","_links":{"self":{"href":"https://api.github.com/repos/jenkinsci/jenkins/pulls/comments/27131567"},"html":{"href":"https://github.com/jenkinsci/jenkins/pull/1596#discussion_r27131567"},"pull_request":{"href":"https://api.github.com/repos/jenkinsci/jenkins/pulls/1596"}}},"pull_request":{"url":"https://api.github.com/repos/jenkinsci/jenkins/pulls/1596","id":30750013,"html_url":"https://github.com/jenkinsci/jenkins/pull/1596","diff_url":"https://github.com/jenkinsci/jenkins/pull/1596.diff","patch_url":"https://github.com/jenkinsci/jenkins/pull/1596.patch","issue_url":"https://api.github.com/repos/jenkinsci/jenkins/issues/1596","number":1596,"state":"open","locked":false,"title":"[JENKINS-27565] Fix threading issues with Nodes and Queue","user":{"login":"stephenc","id":209336,"avatar_url":"https://avatars.githubusercontent.com/u/209336?v=3","gravatar_id":"","url":"https://api.github.com/users/stephenc","html_url":"https://github.com/stephenc","followers_url":"https://api.github.com/users/stephenc/followers","following_url":"https://api.github.com/users/stephenc/following{/other_user}","gists_url":"https://api.github.com/users/stephenc/gists{/gist_id}","starred_url":"https://api.github.com/users/stephenc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stephenc/subscriptions","organizations_url":"https://api.github.com/users/stephenc/orgs","repos_url":"https://api.github.com/users/stephenc/repos","events_url":"https://api.github.com/users/stephenc/events{/privacy}","received_events_url":"https://api.github.com/users/stephenc/received_events","type":"User","site_admin":false},"body":"[JENKINS-27565](https://issues.jenkins-ci.org/browse/JENKINS-27565)\r\n\r\nIn order to schedule work correctly, the Queue needs to have a consistent view of all nodes that are allocated to Jenkins. Similarly, in order to determine which nodes are idle and can thus be removed, the Nodes and Clouds need to have a consistent view of the Queue.\r\n\r\nWhat this all boils down to is that there needs to be a lock to guard critical access to the list of nodes and the actual state of the queue. For UI display purposes there needs to be a path that returns snapshot views, but the code paths that intend to modify the nodes or assign jobs to nodes needs a consistent state.\r\n\r\nTesting these issues require pushing Jenkins in terms of using a large number of nodes that are provisioned and deprovisioned through the cloud api. Failure modes that have been observed:\r\n\r\n* Jobs starting on a node that is removed before the job starts\r\n* Two jobs running on the same node at the same time when the node has one and only one executor and these are not flyweight tasks\r\n* Deadlocks and slow UI when attempting to fix these issues with the existing locks (i.e. the intrinsic locks on `Jenkins.instance` and `Jenkins.instance.queue`)\r\n\r\nThe basic plan for this pull request is to replace the locks used to manage the queue and the list of nodes with a `ReadWriteLock` or similar and proactively produce Snapshot views of the queue and list of nodes so that the UI threads do not even have to acquire the read lock.\r\n\r\n# Plan of attack\r\n\r\n- [x] Pick a recent LTS to base development against (`1.596`)\r\n- [x] Fork branch off mainline release LTS was based off (not the LTS itself as that will complicate merging to master)\r\n- [x] Assign a version number for this branch (`1.596-threadsafe-1-SNAPSHOT`)\r\n- [x] ...\r\n- [x] Move slaves out of global config.xml into a config file per slave\r\n- [x] lots of hacking\r\n- [x] ...\r\n- [x] Revert the version number change to make the merge back to mainline possible\r\n- [x] Merge with `master` branch\r\n- [x] Remove `[WIP]` from pull request\r\n- [ ] Merge to master\r\n","created_at":"2015-03-09T10:39:22Z","updated_at":"2015-03-25T15:27:50Z","closed_at":null,"merged_at":null,"merge_commit_sha":"75ad10c4258dabc3a3b53bbb25842e5e7736a850","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/jenkinsci/jenkins/pulls/1596/commits","review_comments_url":"https://api.github.com/repos/jenkinsci/jenkins/pulls/1596/comments","review_comment_url":"https://api.github.com/repos/jenkinsci/jenkins/pulls/comments{/number}","comments_url":"https://api.github.com/repos/jenkinsci/jenkins/issues/1596/comments","statuses_url":"https://api.github.com/repos/jenkinsci/jenkins/statuses/89835c85acff5832631bb1dfd8e4003a8d444c02","head":{"label":"stephenc:threadsafe-node-queue","ref":"threadsafe-node-queue","sha":"89835c85acff5832631bb1dfd8e4003a8d444c02","user":{"login":"stephenc","id":209336,"avatar_url":"https://avatars.githubusercontent.com/u/209336?v=3","gravatar_id":"","url":"https://api.github.com/users/stephenc","html_url":"https://github.com/stephenc","followers_url":"https://api.github.com/users/stephenc/followers","following_url":"https://api.github.com/users/stephenc/following{/other_user}","gists_url":"https://api.github.com/users/stephenc/gists{/gist_id}","starred_url":"https://api.github.com/users/stephenc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stephenc/subscriptions","organizations_url":"https://api.github.com/users/stephenc/orgs","repos_url":"https://api.github.com/users/stephenc/repos","events_url":"https://api.github.com/users/stephenc/events{/privacy}","received_events_url":"https://api.github.com/users/stephenc/received_events","type":"User","site_admin":false},"repo":{"id":1399992,"name":"jenkins","full_name":"stephenc/jenkins","owner":{"login":"stephenc","id":209336,"avatar_url":"https://avatars.githubusercontent.com/u/209336?v=3","gravatar_id":"","url":"https://api.github.com/users/stephenc","html_url":"https://github.com/stephenc","followers_url":"https://api.github.com/users/stephenc/followers","following_url":"https://api.github.com/users/stephenc/following{/other_user}","gists_url":"https://api.github.com/users/stephenc/gists{/gist_id}","starred_url":"https://api.github.com/users/stephenc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stephenc/subscriptions","organizations_url":"https://api.github.com/users/stephenc/orgs","repos_url":"https://api.github.com/users/stephenc/repos","events_url":"https://api.github.com/users/stephenc/events{/privacy}","received_events_url":"https://api.github.com/users/stephenc/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/stephenc/jenkins","description":"Jenkins Continuous Integration Server","fork":true,"url":"https://api.github.com/repos/stephenc/jenkins","forks_url":"https://api.github.com/repos/stephenc/jenkins/forks","keys_url":"https://api.github.com/repos/stephenc/jenkins/keys{/key_id}","collaborators_url":"https://api.github.com/repos/stephenc/jenkins/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/stephenc/jenkins/teams","hooks_url":"https://api.github.com/repos/stephenc/jenkins/hooks","issue_events_url":"https://api.github.com/repos/stephenc/jenkins/issues/events{/number}","events_url":"https://api.github.com/repos/stephenc/jenkins/events","assignees_url":"https://api.github.com/repos/stephenc/jenkins/assignees{/user}","branches_url":"https://api.github.com/repos/stephenc/jenkins/branches{/branch}","tags_url":"https://api.github.com/repos/stephenc/jenkins/tags","blobs_url":"https://api.github.com/repos/stephenc/jenkins/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/stephenc/jenkins/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/stephenc/jenkins/git/refs{/sha}","trees_url":"https://api.github.com/repos/stephenc/jenkins/git/trees{/sha}","statuses_url":"https://api.github.com/repos/stephenc/jenkins/statuses/{sha}","languages_url":"https://api.github.com/repos/stephenc/jenkins/languages","stargazers_url":"https://api.github.com/repos/stephenc/jenkins/stargazers","contributors_url":"https://api.github.com/repos/stephenc/jenkins/contributors","subscribers_url":"https://api.github.com/repos/stephenc/jenkins/subscribers","subscription_url":"https://api.github.com/repos/stephenc/jenkins/subscription","commits_url":"https://api.github.com/repos/stephenc/jenkins/commits{/sha}","git_commits_url":"https://api.github.com/repos/stephenc/jenkins/git/commits{/sha}","comments_url":"https://api.github.com/repos/stephenc/jenkins/comments{/number}","issue_comment_url":"https://api.github.com/repos/stephenc/jenkins/issues/comments{/number}","contents_url":"https://api.github.com/repos/stephenc/jenkins/contents/{+path}","compare_url":"https://api.github.com/repos/stephenc/jenkins/compare/{base}...{head}","merges_url":"https://api.github.com/repos/stephenc/jenkins/merges","archive_url":"https://api.github.com/repos/stephenc/jenkins/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/stephenc/jenkins/downloads","issues_url":"https://api.github.com/repos/stephenc/jenkins/issues{/number}","pulls_url":"https://api.github.com/repos/stephenc/jenkins/pulls{/number}","milestones_url":"https://api.github.com/repos/stephenc/jenkins/milestones{/number}","notifications_url":"https://api.github.com/repos/stephenc/jenkins/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/stephenc/jenkins/labels{/name}","releases_url":"https://api.github.com/repos/stephenc/jenkins/releases{/id}","created_at":"2011-02-22T23:22:48Z","updated_at":"2015-03-16T11:55:38Z","pushed_at":"2015-03-25T15:27:18Z","git_url":"git://github.com/stephenc/jenkins.git","ssh_url":"git@github.com:stephenc/jenkins.git","clone_url":"https://github.com/stephenc/jenkins.git","svn_url":"https://github.com/stephenc/jenkins","homepage":"http://jenkins-ci.org/","size":81611,"stargazers_count":1,"watchers_count":1,"language":"Java","has_issues":false,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"jenkinsci:master","ref":"master","sha":"9ac2257d0a3fa86928267a82d69d55c5a4e423c1","user":{"login":"jenkinsci","id":107424,"avatar_url":"https://avatars.githubusercontent.com/u/107424?v=3","gravatar_id":"","url":"https://api.github.com/users/jenkinsci","html_url":"https://github.com/jenkinsci","followers_url":"https://api.github.com/users/jenkinsci/followers","following_url":"https://api.github.com/users/jenkinsci/following{/other_user}","gists_url":"https://api.github.com/users/jenkinsci/gists{/gist_id}","starred_url":"https://api.github.com/users/jenkinsci/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jenkinsci/subscriptions","organizations_url":"https://api.github.com/users/jenkinsci/orgs","repos_url":"https://api.github.com/users/jenkinsci/repos","events_url":"https://api.github.com/users/jenkinsci/events{/privacy}","received_events_url":"https://api.github.com/users/jenkinsci/received_events","type":"Organization","site_admin":false},"repo":{"id":1103607,"name":"jenkins","full_name":"jenkinsci/jenkins","owner":{"login":"jenkinsci","id":107424,"avatar_url":"https://avatars.githubusercontent.com/u/107424?v=3","gravatar_id":"","url":"https://api.github.com/users/jenkinsci","html_url":"https://github.com/jenkinsci","followers_url":"https://api.github.com/users/jenkinsci/followers","following_url":"https://api.github.com/users/jenkinsci/following{/other_user}","gists_url":"https://api.github.com/users/jenkinsci/gists{/gist_id}","starred_url":"https://api.github.com/users/jenkinsci/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jenkinsci/subscriptions","organizations_url":"https://api.github.com/users/jenkinsci/orgs","repos_url":"https://api.github.com/users/jenkinsci/repos","events_url":"https://api.github.com/users/jenkinsci/events{/privacy}","received_events_url":"https://api.github.com/users/jenkinsci/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/jenkinsci/jenkins","description":"Jenkins Continuous Integration Server","fork":true,"url":"https://api.github.com/repos/jenkinsci/jenkins","forks_url":"https://api.github.com/repos/jenkinsci/jenkins/forks","keys_url":"https://api.github.com/repos/jenkinsci/jenkins/keys{/key_id}","collaborators_url":"https://api.github.com/repos/jenkinsci/jenkins/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/jenkinsci/jenkins/teams","hooks_url":"https://api.github.com/repos/jenkinsci/jenkins/hooks","issue_events_url":"https://api.github.com/repos/jenkinsci/jenkins/issues/events{/number}","events_url":"https://api.github.com/repos/jenkinsci/jenkins/events","assignees_url":"https://api.github.com/repos/jenkinsci/jenkins/assignees{/user}","branches_url":"https://api.github.com/repos/jenkinsci/jenkins/branches{/branch}","tags_url":"https://api.github.com/repos/jenkinsci/jenkins/tags","blobs_url":"https://api.github.com/repos/jenkinsci/jenkins/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/jenkinsci/jenkins/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/jenkinsci/jenkins/git/refs{/sha}","trees_url":"https://api.github.com/repos/jenkinsci/jenkins/git/trees{/sha}","statuses_url":"https://api.github.com/repos/jenkinsci/jenkins/statuses/{sha}","languages_url":"https://api.github.com/repos/jenkinsci/jenkins/languages","stargazers_url":"https://api.github.com/repos/jenkinsci/jenkins/stargazers","contributors_url":"https://api.github.com/repos/jenkinsci/jenkins/contributors","subscribers_url":"https://api.github.com/repos/jenkinsci/jenkins/subscribers","subscription_url":"https://api.github.com/repos/jenkinsci/jenkins/subscription","commits_url":"https://api.github.com/repos/jenkinsci/jenkins/commits{/sha}","git_commits_url":"https://api.github.com/repos/jenkinsci/jenkins/git/commits{/sha}","comments_url":"https://api.github.com/repos/jenkinsci/jenkins/comments{/number}","issue_comment_url":"https://api.github.com/repos/jenkinsci/jenkins/issues/comments{/number}","contents_url":"https://api.github.com/repos/jenkinsci/jenkins/contents/{+path}","compare_url":"https://api.github.com/repos/jenkinsci/jenkins/compare/{base}...{head}","merges_url":"https://api.github.com/repos/jenkinsci/jenkins/merges","archive_url":"https://api.github.com/repos/jenkinsci/jenkins/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/jenkinsci/jenkins/downloads","issues_url":"https://api.github.com/repos/jenkinsci/jenkins/issues{/number}","pulls_url":"https://api.github.com/repos/jenkinsci/jenkins/pulls{/number}","milestones_url":"https://api.github.com/repos/jenkinsci/jenkins/milestones{/number}","notifications_url":"https://api.github.com/repos/jenkinsci/jenkins/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/jenkinsci/jenkins/labels{/name}","releases_url":"https://api.github.com/repos/jenkinsci/jenkins/releases{/id}","created_at":"2010-11-22T21:21:23Z","updated_at":"2015-03-25T12:40:09Z","pushed_at":"2015-03-25T15:27:23Z","git_url":"git://github.com/jenkinsci/jenkins.git","ssh_url":"git@github.com:jenkinsci/jenkins.git","clone_url":"https://github.com/jenkinsci/jenkins.git","svn_url":"https://github.com/jenkinsci/jenkins","homepage":"http://jenkins-ci.org/","size":400281,"stargazers_count":3838,"watchers_count":3838,"language":"Java","has_issues":false,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":1734,"mirror_url":null,"open_issues_count":114,"forks":1734,"open_issues":114,"watchers":3838,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/jenkinsci/jenkins/pulls/1596"},"html":{"href":"https://github.com/jenkinsci/jenkins/pull/1596"},"issue":{"href":"https://api.github.com/repos/jenkinsci/jenkins/issues/1596"},"comments":{"href":"https://api.github.com/repos/jenkinsci/jenkins/issues/1596/comments"},"review_comments":{"href":"https://api.github.com/repos/jenkinsci/jenkins/pulls/1596/comments"},"review_comment":{"href":"https://api.github.com/repos/jenkinsci/jenkins/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/jenkinsci/jenkins/pulls/1596/commits"},"statuses":{"href":"https://api.github.com/repos/jenkinsci/jenkins/statuses/89835c85acff5832631bb1dfd8e4003a8d444c02"}}}}