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

[FLINK-36645] [flink-autoscaler] Gracefully handle null execution pla… #930

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

sharath1709
Copy link
Contributor

…n from autoscaler

What is the purpose of the change

This pull request makes change to gracefully handle null execution plan returned from the Flink Rest API on autoscaler. This scenario can happen when a rescale operation is already in progress.

Brief change log

  • Gracefully handle null execution plan from autoscaler

Verifying this change

This change added a unit test case to verify the ScalingMetricCollector throws NotReadyException when the execution plan is empty

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

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

Documentation

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

@@ -150,6 +151,11 @@ public static JobTopology fromJsonPlan(
ObjectNode plan = objectMapper.readValue(jsonPlan, ObjectNode.class);
ArrayNode nodes = (ArrayNode) plan.get("nodes");

if (nodes == null || nodes.isEmpty()) {
throw new NotReadyException(
new RuntimeException("No nodes found in the plan, job is not ready yet"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we maybe add a new constructor to the NotReadyException so we don't need to wrap an unnecessary RuntimeException?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the quick review! Updated.

@gyfora gyfora merged commit dfd66c2 into apache:main Jan 13, 2025
104 of 107 checks passed
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