{"action":"created","issue":{"url":"https://api.github.com/repos/apache/spark/issues/4168","labels_url":"https://api.github.com/repos/apache/spark/issues/4168/labels{/name}","comments_url":"https://api.github.com/repos/apache/spark/issues/4168/comments","events_url":"https://api.github.com/repos/apache/spark/issues/4168/events","html_url":"https://github.com/apache/spark/pull/4168","id":55239205,"number":4168,"title":"SPARK-4136. Under dynamic allocation, cancel outstanding executor requests when no longer needed","user":{"login":"sryza","id":654855,"avatar_url":"https://avatars.githubusercontent.com/u/654855?v=3","gravatar_id":"","url":"https://api.github.com/users/sryza","html_url":"https://github.com/sryza","followers_url":"https://api.github.com/users/sryza/followers","following_url":"https://api.github.com/users/sryza/following{/other_user}","gists_url":"https://api.github.com/users/sryza/gists{/gist_id}","starred_url":"https://api.github.com/users/sryza/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sryza/subscriptions","organizations_url":"https://api.github.com/users/sryza/orgs","repos_url":"https://api.github.com/users/sryza/repos","events_url":"https://api.github.com/users/sryza/events{/privacy}","received_events_url":"https://api.github.com/users/sryza/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":33,"created_at":"2015-01-23T02:55:35Z","updated_at":"2015-02-10T01:09:07Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/apache/spark/pulls/4168","html_url":"https://github.com/apache/spark/pull/4168","diff_url":"https://github.com/apache/spark/pull/4168.diff","patch_url":"https://github.com/apache/spark/pull/4168.patch"},"body":"This takes advantage of the changes made in SPARK-4337 to cancel pending requests to YARN when they are no longer needed.\r\n\r\nEach time the timer in `ExecutorAllocationManager` strikes, we compute `maxNumNeededExecutors`, the maximum number of executors we could fill with the current load.  This is calculated as the total number of running and pending tasks divided by the number of cores per executor.  If `maxNumNeededExecutors` is below the total number of running and pending executors, we call `requestTotalExecutors(maxNumNeededExecutors)` to let the cluster manager know that it should cancel any pending requests above this amount.  If not, `maxNumNeededExecutors` is just used as a bound in alongside the configured `maxExecutors` to limit the number of new requests.\r\n\r\nThe patch modifies the API exposed by `ExecutorAllocationClient` for requesting additional executors by moving from `requestExecutors` to `requestTotalExecutors`.  This makes the communication between the `ExecutorAllocationManager` and the `YarnAllocator` easier to reason about and removes some state that needed to be kept in the `CoarseGrainedSchedulerBackend`.  I think an argument can be made that this makes for a less attractive user-facing API in `SparkContext`, but I'm having trouble envisioning situations where a user would want to use either of these APIs.\r\n\r\nThis will likely break some tests, but I wanted to get feedback on the approach before adding tests and polishing."},"comment":{"url":"https://api.github.com/repos/apache/spark/issues/comments/73625504","html_url":"https://github.com/apache/spark/pull/4168#issuecomment-73625504","issue_url":"https://api.github.com/repos/apache/spark/issues/4168","id":73625504,"user":{"login":"andrewor14","id":2133137,"avatar_url":"https://avatars.githubusercontent.com/u/2133137?v=3","gravatar_id":"","url":"https://api.github.com/users/andrewor14","html_url":"https://github.com/andrewor14","followers_url":"https://api.github.com/users/andrewor14/followers","following_url":"https://api.github.com/users/andrewor14/following{/other_user}","gists_url":"https://api.github.com/users/andrewor14/gists{/gist_id}","starred_url":"https://api.github.com/users/andrewor14/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andrewor14/subscriptions","organizations_url":"https://api.github.com/users/andrewor14/orgs","repos_url":"https://api.github.com/users/andrewor14/repos","events_url":"https://api.github.com/users/andrewor14/events{/privacy}","received_events_url":"https://api.github.com/users/andrewor14/received_events","type":"User","site_admin":false},"created_at":"2015-02-10T01:09:06Z","updated_at":"2015-02-10T01:09:06Z","body":"@sryza I may be missing something but I don't understand why `numPendingExecutors` should be rederived from the user calling `requestTotalExecutors` every time. What exactly was the issue with decrementing it as we did before? Also I believe the current code is incorrect because the user (or us internally) may call either `requestExecutor` or `requestTotalExecutors`, and now we only reset it for the latter. If we were to do this we should do it in `doRequestTotalExecutors` instead."}}