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

Support aggregated endpoint for customized stoppable check #2919

Merged

Conversation

MarkGaox
Copy link
Contributor

@MarkGaox MarkGaox commented Sep 12, 2024

Issues

Description

  • Here are some details about my PR, including screenshots of any UI changes:
  • This PR supports the aggregated customized stoppable check.
  • When configuring the RESTConfig with an specific URI (e.g., "https://*:123"), users means to use the aggregated health check instead of the instance/partition level checks.
  • The aggregated health check will pass instances, toBeStoppedInstances, and the clusterID to the health check endpoint and expect the user to return the response regarding whether those instances are stoppable and the reason of being not stoppable.
  • Support Redirect of request if receiving status code of 302 with max retry of 3 times.
  • Following is an expected request + response:
     example url: POST http://<baseUrl>/aggregatedHealthStatus -d {
       "instances" : ["n1", "n3", "n5", "n9"],
       "to_be_stopped_instances": "["n2", "n4"]",
       "cluster_id": "cluster1",
       "<key>": "<value>"
     ...
     }
     example response:
     {
       "stoppableInstances": ["n1", "n3"],
       "nonStoppableInstancesWithReasons": {
       "n5": "reason1,reason2",
       "n9": "reason3"
     }

Tests

  • The following tests are written for this issue:
mvn test -Dtest=TestInstancesAccessor,TestMaintenanceManagementService,TestInstanceValidationUtilInRest,TestPerInstanceAccessor,TestCustomRestClient -pl helix-rest && mvn test -Dtest=TestInstanceValidationUtil -pl helix-core

Changes that Break Backward Compatibility (Optional)

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

(Consider including all behavior changes for public methods or API. Also include these changes in merge description so that other developers are aware of these changes. This allows them to make relevant code changes in feature branches accounting for the new method/API behavior.)

Documentation (Optional)

  • In case of new functionality, my PR adds documentation in the following wiki page:

(Link the GitHub wiki you added)

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
Copy link
Contributor

xyuanlu commented Sep 15, 2024

could you please also update naming in pr?

@MarkGaox MarkGaox changed the title Support cluster level customized stoppable check Support aggregated endpoint for customized stoppable check Sep 15, 2024
@xyuanlu
Copy link
Contributor

xyuanlu commented Sep 19, 2024

Also could you please update the description with new naming? Thx 😄

Copy link
Contributor

@zpinto zpinto left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the change :)

Copy link
Contributor

@junkaixue junkaixue left a comment

Choose a reason for hiding this comment

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

I don't have major concerns. But take a look of failure tests.

@MarkGaox
Copy link
Contributor Author

MarkGaox commented Sep 25, 2024

I don't have major concerns. But take a look of failure tests.

The failed test passed locally.

Screenshot 2024-09-25 at 1 30 42 AM

@MarkGaox
Copy link
Contributor Author

This PR is approved by @zpinto and @junkaixue. TFTR. It's ready to be merged. The commit message: "Support aggregated endpoint for customized stoppable check"

@junkaixue junkaixue merged commit 2de4177 into apache:master Sep 25, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants