Return current Evacuation status of isEvacuateFinished #100

LZD-PratyushBhatt · 2026-01-09T05:22:50Z

Issues

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

Return remaining paritition count during isEvacuateFinished

(apache#200 - Link your issue number here: You can write "Fixes #XXX". Please use the proper keyword so that the issue gets closed automatically. See https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue
Any of the following keywords can be used: close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved)

Description

Currently isEvacuateFinished API just returns a boolean response, users have no visibility into why the evacuation is blocked. This PR adds a remainingPartitionCounter, which returns the client remaining count and failure reason as well.

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

(Write a concise description including what, why, how)

Tests

Before:

{
  "successful": true
}

New respons e structure examples:

IN_PROGRESS

{
  "successful": true,
  "state": "COMPLETED",
  "remainingPartitionCount": 0,
  "pendingMessageCount": 0,
  "lastActivityTimestamp": 1736812345678
}

NOT_EVACUATING

{
  "successful": false,
  "state": "NOT_EVACUATING",
  "reason": "Instance is not in EVACUATE operation"
}

New tests added.

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:

(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)

helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java

helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PerInstanceAccessor.java

ngngwr · 2026-01-12T05:16:51Z

@LZD-PratyushBhatt Can you update the PR description test section with before and new responses as well. Makes it clear.

helix-core/src/main/java/org/apache/helix/model/EvacuationInfo.java

LZD-PratyushBhatt · 2026-01-12T06:25:28Z

Review comment from @bellatrix007
"One major thing, i wanted to check is, can we also propagate the last timestamp when things changed while evacuating the partitions?"

LZD-PratyushBhatt · 2026-01-12T06:41:01Z

Review comment from @bellatrix007 "One major thing, i wanted to check is, can we also propagate the last timestamp when things changed while evacuating the partitions?"

One way I can think of achieving this is to iterate over all the currentState records(as for each resource its a diff znode) and get the mtime, pick the latest one among all. I'll check this one out, it shouldn't be extra zk calls as we are already iterating over all the currentStates already.

LZD-PratyushBhatt · 2026-01-12T08:36:06Z

Review comment from @bellatrix007 "One major thing, i wanted to check is, can we also propagate the last timestamp when things changed while evacuating the partitions?"

One way I can think of achieving this is to iterate over all the currentState records(as for each resource its a diff znode) and get the mtime, pick the latest one among all. I'll check this one out, it shouldn't be extra zk calls as we are already iterating over all the currentStates already.

Updated with using getPropertyStats, the current code uses getChildValues which doesnt return stats, as the method internally passes null stat object so zk ultimately doesnt populate it.

helix-core/src/main/java/org/apache/helix/model/EvacuationInfo.java

helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java

ngngwr · 2026-01-14T02:05:19Z

helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java

+    EvacuationInfo result = new EvacuationInfo();
+
+    InstanceConfig config = getInstanceConfig(clusterName, instanceName);
+    if (config == null || config.getInstanceOperation().getOperation() !=


I believe we should have different reason code for config as null vs no evacuate operation assigned.

I dont think so it will add any value, even if instance config is not present, instance is technically not evacauting, and in both cases the user will anyways go to the cluster to check the instance config.

At some point, we will propagate this to UI. For user facing things, we should be meticulous about what messages we show/return.

I think no harm in adding it, added! Thanks for the suggestion!

ngngwr · 2026-01-14T02:15:24Z

helix-rest/src/test/java/org/apache/helix/rest/server/TestPerInstanceAccessor.java

-    Assert.assertTrue(evacuateFinishedResult.get("successful"));
+    Assert.assertTrue((Boolean) evacuateFinishedResult.get("successful"));
+    // Verify new fields are present in the response
+    Assert.assertTrue(evacuateFinishedResult.containsKey("remainingCount"),


*remainingPartitionCount

ngngwr · 2026-01-14T02:16:52Z

helix-rest/src/test/java/org/apache/helix/rest/server/TestPerInstanceAccessor.java

+        "Response should contain remainingCount field");
+    Assert.assertTrue(evacuateFinishedResult.containsKey("pendingMessageCount"),
+        "Response should contain pendingMessageCount field");
+    Assert.assertEquals(evacuateFinishedResult.get("remainingCount"), 0,


Please change here as well - *remainingCount

line apache#788 as well

ngngwr · 2026-01-14T02:18:13Z

helix-rest/src/test/java/org/apache/helix/rest/server/TestPerInstanceAccessor.java

+        "Response should contain remainingCount field");
+    Assert.assertTrue(evacuateFinishedResult.containsKey("pendingMessageCount"),
+        "Response should contain pendingMessageCount field");
+    Assert.assertEquals(evacuateFinishedResult.get("remainingCount"), 0,


line apache#788 as well

ngngwr · 2026-01-14T02:20:20Z

helix-rest/src/test/java/org/apache/helix/rest/server/TestPerInstanceAccessor.java

    evacuateFinishedResult = OBJECT_MAPPER.readValue(response.readEntity(String.class), Map.class);
    Assert.assertEquals(response.getStatus(), Response.Status.OK.getStatusCode());
-    Assert.assertTrue(evacuateFinishedResult.get("successful"));
+    Assert.assertTrue((Boolean) evacuateFinishedResult.get("successful"));


Are we still keeping "successful"? Make sense to ensure backward compatibility. But I do not see this in tests responses shared in PR description. Can you please update those.

Please update the final responses after lastActivityAddition?

ngngwr

Thanks for doing the changes. LGTM.

LZD-PratyushBhatt added 3 commits January 9, 2026 09:02

Return remaining paritition count during isEvacuateFinished

d11955e

Refactor

fc50a57

Add tests

a2a090c

LZD-PratyushBhatt changed the title ~~Return remaining paritition count during isEvacuateFinished~~ Return current Evacuation status of isEvacuateFinished Jan 12, 2026

vivek8420 reviewed Jan 12, 2026

View reviewed changes

helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java Outdated Show resolved Hide resolved

vivek8420 reviewed Jan 12, 2026

View reviewed changes

helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java Outdated Show resolved Hide resolved

Chr0nicl3 reviewed Jan 12, 2026

View reviewed changes

helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java Outdated Show resolved Hide resolved

Chr0nicl3 reviewed Jan 12, 2026

View reviewed changes

helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java Outdated Show resolved Hide resolved

helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java Outdated Show resolved Hide resolved

ngngwr reviewed Jan 12, 2026

View reviewed changes

helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PerInstanceAccessor.java Show resolved Hide resolved

ngngwr reviewed Jan 12, 2026

View reviewed changes

helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PerInstanceAccessor.java Show resolved Hide resolved

ADdress review comments

29847d8

ngngwr reviewed Jan 12, 2026

View reviewed changes

helix-core/src/main/java/org/apache/helix/model/EvacuationInfo.java Outdated Show resolved Hide resolved

LZD-PratyushBhatt added 2 commits January 12, 2026 11:16

ADdress review comments 2

740cdf8

Use Integer instead of primitive int

ae527df

LZD-PratyushBhatt requested review from Chr0nicl3, ngngwr, thestreak101 and vivek8420 January 12, 2026 06:12

Add LastModifiedTime field for more visibility

8a73eb1

kabragaurav reviewed Jan 12, 2026

View reviewed changes

helix-core/src/main/java/org/apache/helix/model/EvacuationInfo.java Show resolved Hide resolved

LZD-PratyushBhatt requested a review from kabragaurav January 12, 2026 16:19

vivek8420 reviewed Jan 13, 2026

View reviewed changes

helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java Outdated Show resolved Hide resolved

Use stream for getting the lastModifiedTime

7c6b212

LZD-PratyushBhatt requested a review from vivek8420 January 13, 2026 07:26

ngngwr reviewed Jan 14, 2026

View reviewed changes

Address review comments

8517f72

LZD-PratyushBhatt requested a review from ngngwr January 14, 2026 03:43

vivek8420 approved these changes Jan 14, 2026

View reviewed changes

Add diff reason code

fc6c165

ngngwr approved these changes Jan 14, 2026

View reviewed changes

LZD-PratyushBhatt merged commit f96c61a into dev Jan 14, 2026
1 of 2 checks passed

Return current Evacuation status of isEvacuateFinished #100

Return current Evacuation status of isEvacuateFinished #100

Uh oh!

Conversation

LZD-PratyushBhatt commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issues

Description

Tests

Changes that Break Backward Compatibility (Optional)

Documentation (Optional)

Commits

Code Quality

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ngngwr commented Jan 12, 2026

Uh oh!

Uh oh!

LZD-PratyushBhatt commented Jan 12, 2026

Uh oh!

LZD-PratyushBhatt commented Jan 12, 2026

Uh oh!

LZD-PratyushBhatt commented Jan 12, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngngwr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

LZD-PratyushBhatt commented Jan 9, 2026 •

edited

Loading