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

Add typoed name DataPropagationLatencyGuage as deprecated #517

Merged
merged 2 commits into from
Oct 23, 2019

Conversation

huizhilu
Copy link
Contributor

@huizhilu huizhilu commented Oct 21, 2019

Issues

  • My PR addresses the following Helix issues and references them in the PR title:
    Name DataPropagationLatencyGauge has a typo #512
    If we remove the typoed name DataPropagationLatencyGuage, current metrics graph may not see the metric and historical DataPropagationLatencyGuage data may be lost. To solve the problem, we need to add back DataPropagationLatencyGuage and set it as deprecated.

Description

  • Here are some details about my PR, including screenshots of any UI changes:
    If we remove the typoed name DataPropagationLatencyGuage, current metrics graph may not see the metric and historical DataPropagationLatencyGuage data may be lost. To solve the problem, we need to add back DataPropagationLatencyGuage and set it as deprecated.

Tests

  • [] The following tests are written for this issue:
    (List the names of added unit/integration tests)

  • The following is the result of the "mvn test" command on the appropriate module:

Tests all pass.

[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] TestPartitionLevelTransitionConstraint.test:160 expected: but was:
[ERROR] TestTaskPerformanceMetrics.testTaskPerformanceMetrics:118 expected: but was:
[INFO]
[ERROR] Tests run: 882, Failures: 2, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 53:55 min
[INFO] Finished at: 2019-10-21T22:41:58-07:00

END TestPartitionLevelTransitionConstraint_test at Mon Oct 21 22:47:22 PDT 2019
END test at Mon Oct 21 22:47:22 PDT 2019, took: 1650ms.
Shut down zookeeper at port 2183 in thread main
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 6.305 s - in org.apache.helix.integration.TestPartitionLevelTransitionConstraint
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

TestTaskPerformanceMetrics#testTaskPerformanceMetrics
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.152 s - in org.apache.helix.monitoring.mbeans.TestTaskPerformanceMetrics
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
[INFO]

Commits

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

    In case of new functionality, my PR adds documentation in the following wiki page:
    (Link the GitHub wiki you added)

Code Quality

  • My diff has been formatted using helix-style-intellij.xml

Copy link
Contributor

@narendly narendly left a comment

Choose a reason for hiding this comment

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

Please address my comments. Looks good. Thanks for adding it back!

@huizhilu
Copy link
Contributor Author

This PR is ready to be merged, approved by @narendly

Final commit comment:
If we remove the typoed name DataPropagationLatencyGuage, current metrics graph may not see the metric and historical DataPropagationLatencyGuage data may be lost. To support backward compatibility, add back DataPropagationLatencyGuage and set it as deprecated.

@narendly narendly merged commit aad78eb into apache:master Oct 23, 2019
narendly pushed a commit to narendly/helix that referenced this pull request Dec 24, 2019
…ark it deprecated (apache#517)

If we remove the name with a typo, DataPropagationLatencyGuage, current metrics graph may not see the old metric and historical DataPropagationLatencyGuage data might get lost. To support backward compatibility, adding back DataPropagationLatencyGuage and mark it as deprecated.
@huizhilu huizhilu deleted the DataPropagationLatencyGauge branch February 11, 2020 08:07
mgao0 pushed a commit to mgao0/helix that referenced this pull request Mar 6, 2020
…ark it deprecated (apache#517)

If we remove the name with a typo, DataPropagationLatencyGuage, current metrics graph may not see the old metric and historical DataPropagationLatencyGuage data might get lost. To support backward compatibility, adding back DataPropagationLatencyGuage and mark it as deprecated.
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.

None yet

2 participants