Skip to content

[ML] Rebalance should not notify listener before update is applied #88012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

dimitris-athanasiou
Copy link
Contributor

When we are rebalancing trained model assignments, we eventually
update cluster state with the new metadata. We only want to
notify the listener after we have applied the cluster state update.
This commit fixes a bug where we could notify before the update
was actually applied resulting in NPE when writing the response
of the CreateTrainedModelAssignmentAction.

This is a fix following the work done in #87366.

When we are rebalancing trained model assignments, we eventually
update cluster state with the new metadata. We only want to
notify the listener after we have applied the cluster state update.
This commit fixes a bug where we could notify before the update
was actually applied resulting in NPE when writing the response
of the `CreateTrainedModelAssignmentAction`.

This is a fix following the work done in elastic#87366.
@elasticmachine elasticmachine added the Team:ML Meta label for the ML team label Jun 24, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

Comment on lines +414 to +416
if (isUpdated) {
listener.onResponse(TrainedModelAssignmentMetadata.fromState(newState));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah, this would happen because of the return currentState in execute kicking off another rebalancing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you were right there was something off there!

@dimitris-athanasiou dimitris-athanasiou merged commit 45c56b2 into elastic:master Jun 24, 2022
@dimitris-athanasiou dimitris-athanasiou deleted the rebalance-should-not-notify-listener-before-update-is-applied branch June 24, 2022 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml Machine learning >non-issue Team:ML Meta label for the ML team v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants