andygrove commented on PR #2477: URL: https://github.com/apache/datafusion-ballista/pull/2477#issuecomment-5783658045
Thanks for tracking this down, the explanation of why the `TaskUpdating` refund never fires after eviction is really clear. One small thing on the description though. `cancel_running_tasks` spawns the RPCs and returns `Ok` right away, so the refund actually happens as soon as the cancel is sent, not after the executors acknowledge it, and a failed cancel RPC still gets refunded. That seems like a fine trade-off to me since the alternative is leaking the vcores forever, but it means an executor can be briefly oversubscribed while the cancelled tasks wind down. Could you update the description (and maybe the code comment in `abort_job`) to reflect that? Also, I checked for double refunds against the `TaskUpdating` path and the serial event loop plus eviction inside `abort_job` rules that out, so no concerns there. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
