[Data][Cherry-pick] Fixed min_scheduling_resources to fallback to incremental_resource_usage by default#60998
Conversation
…urce_usage by default (#60997) ## Description - Fixed min_scheduling_resources to fallback to incremental_resource_usage by default - Explicitly overridden for `HashShufflingOperatorBase` ## Related issues > Link related issues: "Fixes #1234", "Closes #1234", or "Related to #1234". ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: Alexey Kudinkin <ak@anyscale.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the default implementation for min_scheduling_resources in PhysicalOperator to use incremental_resource_usage(). This is a sensible change that provides a better default. My review includes a suggestion to remove a now-redundant override in HashShufflingOperatorBase that was also added, which will simplify the code while retaining the same behavior due to the base class change.
| def min_scheduling_resources(self) -> ExecutionResources: | ||
| return self.incremental_resource_usage() | ||
|
|
There was a problem hiding this comment.
This override of min_scheduling_resources is now redundant. The base class PhysicalOperator has been updated in this pull request to return self.incremental_resource_usage() by default. Since HashShufflingOperatorBase inherits this behavior, this explicit override can be removed to simplify the code.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
3 participants
Description
Cherry-pick of #60997
Related issues
Additional information
Description
Related issues
Additional information