{"action":"created","issue":{"url":"https://api.github.com/repos/apache/spark/issues/9154","labels_url":"https://api.github.com/repos/apache/spark/issues/9154/labels{/name}","comments_url":"https://api.github.com/repos/apache/spark/issues/9154/comments","events_url":"https://api.github.com/repos/apache/spark/issues/9154/events","html_url":"https://github.com/apache/spark/pull/9154","id":111929778,"number":9154,"title":"[SPARK-11163] Remove unnecessary addPendingTask calls.","user":{"login":"kayousterhout","id":1108612,"avatar_url":"https://avatars.githubusercontent.com/u/1108612?v=3","gravatar_id":"","url":"https://api.github.com/users/kayousterhout","html_url":"https://github.com/kayousterhout","followers_url":"https://api.github.com/users/kayousterhout/followers","following_url":"https://api.github.com/users/kayousterhout/following{/other_user}","gists_url":"https://api.github.com/users/kayousterhout/gists{/gist_id}","starred_url":"https://api.github.com/users/kayousterhout/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kayousterhout/subscriptions","organizations_url":"https://api.github.com/users/kayousterhout/orgs","repos_url":"https://api.github.com/users/kayousterhout/repos","events_url":"https://api.github.com/users/kayousterhout/events{/privacy}","received_events_url":"https://api.github.com/users/kayousterhout/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":9,"created_at":"2015-10-16T23:35:10Z","updated_at":"2015-10-19T17:12:15Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/apache/spark/pulls/9154","html_url":"https://github.com/apache/spark/pull/9154","diff_url":"https://github.com/apache/spark/pull/9154.diff","patch_url":"https://github.com/apache/spark/pull/9154.patch"},"body":"This commit removes unnecessary calls to addPendingTask in\r\nTaskSetManager.executorLost. These calls are unnecessary: for\r\ntasks that are still pending and haven't been launched, they're\r\nstill in all of the correct pending lists, so calling addPendingTask\r\nhas no effect. For tasks that are currently running (which may still be\r\nin the pending lists, depending on how they were scheduled), we call\r\naddPendingTask in handleFailedTask, so the calls at the beginning\r\nof executorLost are redundant.\r\n\r\nI think these calls are left over from when we re-computed the locality\r\nlevels in addPendingTask; now that we call recomputeLocality separately,\r\nI don't think these are necessary.\r\n\r\n@markhamstra can you take a look at this?\r\n\r\ncc @vanzin "},"comment":{"url":"https://api.github.com/repos/apache/spark/issues/comments/149284732","html_url":"https://github.com/apache/spark/pull/9154#issuecomment-149284732","issue_url":"https://api.github.com/repos/apache/spark/issues/9154","id":149284732,"user":{"login":"vanzin","id":1694083,"avatar_url":"https://avatars.githubusercontent.com/u/1694083?v=3","gravatar_id":"","url":"https://api.github.com/users/vanzin","html_url":"https://github.com/vanzin","followers_url":"https://api.github.com/users/vanzin/followers","following_url":"https://api.github.com/users/vanzin/following{/other_user}","gists_url":"https://api.github.com/users/vanzin/gists{/gist_id}","starred_url":"https://api.github.com/users/vanzin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vanzin/subscriptions","organizations_url":"https://api.github.com/users/vanzin/orgs","repos_url":"https://api.github.com/users/vanzin/repos","events_url":"https://api.github.com/users/vanzin/events{/privacy}","received_events_url":"https://api.github.com/users/vanzin/received_events","type":"User","site_admin":false},"created_at":"2015-10-19T17:12:15Z","updated_at":"2015-10-19T17:12:15Z","body":"> What do you mean about tasks being in the wrong list?\r\n\r\nLet's say you have a task whose locality preference includes \"executor 1\". Now \"executor 1\" dies. That task would be in the `pendingTasksForExecutor` list for that executor, and now that's wrong and it should be fixed. My understanding is that `addPendingTask` is achieving that here."}}