Skip to content
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

[hotfix] Fix incorrect messageKey passed in ScalingLimited event #919

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

huyuanfeng2018
Copy link
Contributor

@huyuanfeng2018 huyuanfeng2018 commented Nov 25, 2024

What is the purpose of the change

Fix incorrect messageKey passed in ScalingLimited event. This will cause almost every ScalingLimited event to generate a record, scaling.event.interval will not take effect.

Brief change log

Change the calculation method of messagekey to SCALING_LIMITED + vertex + newParallelism

Verifying this change

Added test in JobVertexScalerTest#testSendingScalingLimitedEvents to test the stability of eventKey when scaleFactor changes do not cause parallelism changes.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API, i.e., is any changes to the CustomResourceDescriptors: (yes / no)
  • Core observer or reconciler logic that is regularly executed: (yes / no)

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@gyfora
Copy link
Contributor

gyfora commented Nov 26, 2024

Should we add a test to guard against future regression?

@huyuanfeng2018
Copy link
Contributor Author

Should we add a test to guard against future regression?

Thanks for your quick review, this part of the test has been added

@gyfora gyfora merged commit 4683333 into apache:main Nov 27, 2024
104 checks passed
@huyuanfeng2018 huyuanfeng2018 deleted the hotfix-messagekey branch November 27, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants