tgravescs commented on PR #37268:
URL: https://github.com/apache/spark/pull/37268#issuecomment-1203990274
thanks for adding more details.
```
/**
* Return resource profile Ids of executors where tasks can be assigned to.
*/
def compatibleExecutorRpIds(rpMgr: ResourceProfileManager): Set[Int]
```
It seems a little bit odd to ask the ResourceProfile to give you compatible
other ResourceProfiles. This feels like it should be in the
ResourceProfileManager which knows about all the ResourceProfiles. I guess
that is why you pass in the ResourceProfileManager here? Is the intention
the user could explicitly set which ResourceProfiles its compatible with? If
so I definitely would want a way to not have to specify it.
The other issue raised that wasn't addressed was the reuse policy. I guess
in this case we are limiting the executor profile to 1 because we don't have
dynamic allocation so one could argue that if you use task resource request
with that you know what you get. Which I am fine with but we need to be clear
that it might very well waste resources.
Also if the intent is to not support TaskResourceProfile with dynamic
allocation, I think we should throw an exception if anyone uses it with the
dynamic allocation config on.
--
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]
