Fix endless creation of best possible nodes

[GrantPSpencer]

Issues

• My PR addresses the following Helix issues and references them in the PR description:
  Controller endlessly creates BEST_POSSIBLE nodes due to handleDelayedRebalanceMinActiveReplica #2971

Description

• Here are some details about my PR, including screenshots of any UI changes:
  Controller endlessly creates best possible nodes under certain circumstances due handlehandleDelayedRebalanceMinActiveReplica modifying the same underlying ResourceAssignments that are used for the in memory bestPossibleAssignment

This PR changes AssignmentMetadataStore's methods persistBaseline and persistBestPossibleAssignment to create a copy of the underlying ResourceAssignment objects before persisting them to the store's in-memory map. This allows the object passed to these methods to be safely modified afterwards without affecting the cached bestPossible.

There are specific circumstances to trigger this behavior:

1. Cluster is in state to require delyRebalance overwrites (nodes are disabled within delay window and partitions are violating minActiveReplica count)
2. Change cluster so that partialRebalance will calculate a new assignment (adding or removing instances)

PartialRebalance will async calculate a new best possible and increment the _bestPossibleVersion and schedule an onDemandPipeline run. In emergencyRebalance(...) of the next pipeline, the bestPossibleAssignment (object A) will be persisted as _bestPossibleVersion != _lastPersistedBestPossibleVersion. The values of the new best possible are stored in our in-memory bestPossibleAssignment map (object B). While object A and B are different, their values map to the same objects. We then pass object A to handleDelayedRebalanceMinActiveReplica which modifies the values of object A, also modifying the values of object B. This causes partialRebalance to believe it has calculated a different assignment, so it triggers a new pipeline run which will repeat the above behavior ad infinitum.

Tests

• The following tests are written for this issue:

TestEndlessBestPossibleNodes
This test will fail on master, but pass on this branch

$ mvn test -o -Dtest=TestEndlessBestPossibleNodes -pl=helix-core 

[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 15.512 s - in org.apache.helix.integration.TestEndlessBestPossibleNodes
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  53.849 s
[INFO] Finished at: 2024-11-26T15:52:41-08:00
[INFO] ------------------------------------------------------------------------

Changes that Break Backward Compatibility (Optional)

• My PR contains changes that break backward compatibility or previous assumptions for certain methods or API. They include:
  N/A

Commits

• My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Code Quality

• My diff has been formatted using helix-style.xml
  (helix-style-intellij.xml if IntelliJ IDE is used)

[xyuanlu]

Thanks for offline explain.
LGTM.

[GrantPSpencer]

Pull request approved by: @xyuanlu
Commit message: Fix endless creation of best possible nodes triggered by unintended modification of cached best possible map. The baseline and bestPossible maps are type Map<String, ResourceAssignment>. When this map is stored in the cache by persistBaseline() and persistBestPossibleAssignment(), the ResourceAssignment objects can still be accessed and modified after calling the relevant persist method. This change creates a deep copy of the assignment map with new ResourceAssignment objects to prevent unintended modification of the cached assignment after the call to persist.

[xyuanlu]

This is a bit complex change, even though code change is short, the logic is pretty complex. Could you please add a bit more explain in commit message please? thx