apache / helix

Mirror of Apache Helix
Apache License 2.0
457 stars 218 forks source link

Change partitionAssignment API to handle ANY_LIVEINSTANCE #2817

Closed GrantPSpencer closed 2 weeks ago

GrantPSpencer commented 1 month ago

Issues

2816

partitionAssignment fails to calculate when there is FULL_AUTO resource that has ANY_LIVEINSTANCE for replica count.

Description

partitionAssignment fails due to Integer.parseInt(idealState.getReplicas()) in the getIdealAssignmentForFullAuto method. idealState.getReplicas() returns "ANY_LIVEINSTANCE" which then fails to be parsed to an int.

This change switches to the getReplicaCount() method which takes a default value if the replica count is ANY_LIVEINSTANCE.

Tests

Added a resource that utilizes ANY_LIVEINSTANCE into the testComputePartitionAssignmentMaintenanceMode test method. Without the changes to getReplicas()

[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 147.587 s - in org.apache.helix.rest.server.TestPartitionAssignmentAPI
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] 
[INFO] --- jacoco:0.8.6:report (generate-code-coverage-report) @ helix-rest ---
[INFO] Loading execution data file /Users/gspencer/Desktop/git-repos/helix/helix-rest/target/jacoco.exec
[INFO] Analyzed bundle 'Apache Helix :: Restful Interface' with 95 classes
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  02:34 min
[INFO] Finished at: 2024-06-26T16:23:02-07:00
[INFO] ------------------------------------------------------------------------

Changes that Break Backward Compatibility (Optional)

Change to the return value of getReplicas when called on a resource with ANY_LIVEINSTANCE as its replica count.

Commits

Code Quality

GrantPSpencer commented 3 weeks ago

@junkaixue Updated 2 other areas where getReplicas was called without handling possible number format exception

GrantPSpencer commented 3 weeks ago

Failed due to 2 flaky tests: updateInstance - https://github.com/apache/helix/issues/2744

partitionAssignmentAPI afterTest - https://github.com/apache/helix/issues/2754

Error:  Test failed: updateInstance(org.apache.helix.rest.server.TestPerInstanceAccessor)  Time elapsed: 0.526 s  <<< FAILURE!
Error:  Test failed: afterTest(org.apache.helix.rest.server.TestPartitionAssignmentAPI)  Time elapsed: 12.493 s  <<< FAILURE!
GrantPSpencer commented 3 weeks ago

Pull request approved @junkaixue Commit message: Handle ANY_LIVEINSTANCE by calling getReplicaCount