Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3783#issuecomment-69405655
Hi @zsxwing this is looking pretty close. However, there is still one thing
about the current changes that I'm not sure about, as I have discussed in the
comments inline.
The use of `retain` in `schedule` actually changes the semantics a little.
Before we would always remove `executorId` from `removeTimes` if the time
expired, but now we do this only if we we actually removed the associated
executor. The problem with this is that every time we call schedule we'll keep
trying to remove it and failing and this will lead to a lot of noise. I think
instead we should try to maintain the old semantics as follows:
```
removeTimes.retain { case (executorId, expireTime) =>
val expired = now >= expireTime
if (expired) { removeExecutor(executorId) }
expired
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
