Fix rebalancer failure counter in async scenario

[NealSun96]

Issues

• My PR addresses the following Helix issues and references them in the PR description:

Fixes #2062

Description

• Here are some details about my PR, including screenshots of any UI changes:

Async processes cannot propagate exceptions upwards, so we need to explicitly increment the failure counters during exception handling in the submission block. Since partial rebalance is turning into async, this PR fixes both the old problem (global rebalance missing rebalance failure counter) and the new problem.

Tests

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

(If CI test fails due to known issue, please specify the issue and test PR locally. Then copy & paste the result of "mvn test" to here.)

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)

[qqu0127]

For my own understanding, how was it working in the past? Did we simply skip the error and not counting?

[NealSun96]

Yes, Global Rebalance failure was never accounted for in the failure metric. See #2062 .

[qqu0127]

LGTM, also shall we consider cherry-pick this to the master branch?

[NealSun96]

@qqu0127 This will be merged into master with feature branch, which happens now. :)

[jiajunwang]

If both baseline and partial rebalance fail, will we count 2 times instead of 1? Best Regards, Jiajun

…

On Mon, Dec 12, 2022 at 4:00 PM Neal Sun ***@***.***> wrote: Merged #2318 <#2318> into nealsun/waged-pipeline-redesign. — Reply to this email directly, view it on GitHub <#2318 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANYM2CD4WR435E3WGXRQMTWM64CVANCNFSM6AAAAAAS4PF4OA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[NealSun96]

@jiajunwang missed your message: yes we will count 2 times. Conceptually, given that these pipelines are separate async pipelines now, counting 2 times does make sense (rebalance failed on 2 occasions with possibly different settings). Operationally, since the metric is used to indicate errors, counting 2 times is mostly similar to counting 1 time (and definitely better than counting 0 times).

[jiajunwang]

Agreed. Not a concern.

[desaikomal]

LGTM