Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/19048
> May be it would be cleaner if we provide a new api like this -
killExecutorsAndNotUpdateTotal?
I think the main thing that bothers me is that adding anything to the API
is making all this code even more complicated and confusing than it already is.
Having two (3 if you count the YARN allocator) places track all this state
is bound to lead to these issues. Optimally only the EAM would keep track of
these things; the CGSB shouldn't really be dealing with executor allocation and
de-allocation, just with managing the existing executors that connect to it.
But fixing things like that is probably a much larger change (the words
"hornets' nest" come to mind).
Barring that, I think that we should make the change that leads to the
correct behavior without making the internal interface more complicated than it
needs to be. If changing the semantics of `ExecutorAllocationClient` lead to
the code being easier to follow, then that's what we should do. After all,
there is a single implementation of it (the CGSB). (And, digressing back to my
paragraph above, maybe `ExecutorAllocationClient` shouldn't even exist and we
should only have the EAM. But back to this PR.)
Or maybe you can reach the same thing through other means. For example,
maybe if you get rid of the `replace` argument and make `killExecutors` not
update the CGSB target count, and then force the caller to call
`requestTotalExecutors` before killing executors, you could achieve the same
thing. Maybe there are corner cases doing that, but maybe it works?
If none of those work, then we can talk about adding new things.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
