Closed himanshukandwal closed 4 months ago
Is this just for adding a test?
Is this just for adding a test?
Hey @junkaixue yes, I first reproduced it using the test, now added the fix. Would you pls review it when get a chance.
Please remove WIP from title
Done
This PR has been reviewed and approved by @zpinto.
Final Commit Message: When adding a new WAGED resource with a tag and without any instances against that tag, we are observing NPE coming from the system. To solve this issue we are adding a check in the ResourceComputationStage to have such resources excluded from the pipeline computation and only be considered when there are actual resource partitions (>0) to be assigned to the instances.
lgtm. Let's wait for the tests.
Hey @junkaixue, the PR CI successful.
Also ran the full CI suite in my repo and thats successful as well: https://github.com/himanshukandwal/helix/actions/runs/8471701884/job/23212239715
Issues
Description
numPartitions = 0
.To solve this issue we are adding a check in the
ResourceComputationStage
to have such resources excluded from the pipeline computation and only be considered when there are actual resource partitions (>0
) to be assigned to the instances.Tests
(List the names of added unit/integration tests)
TestWagedClusterExpansionWithAddingResourcesBeforeInstances.testExpandClusterWithResourceWithoutInstances
The following is the result of the "mvn test" command on the appropriate module:
Changes that Break Backward Compatibility (Optional)
(Consider including all behavior changes for public methods or API. Also include these changes in merge description so that other developers are aware of these changes. This allows them to make relevant code changes in feature branches accounting for the new method/API behavior.)
Documentation (Optional)
(Link the GitHub wiki you added)
Commits
Code Quality