Consolidate two disk util rebalance prechecks into one by J-HowHuang · Pull Request #19176 · apache/pinot

J-HowHuang · 2026-08-06T23:42:27Z

Description

Originally the disk utilization precheck in table rebalance is easily confusing. Reduce the unnecessary information so that it's more intuitive.

Original Behavior

Precheck diskUtilizationDuringRebalance: shows the highest disk utilization on each server it could get during the rebalance (i.e. the footprint, the worst case if all segments are added first, then removed)
Precheck diskUtilizationAfterRebalance: shows the net disk utilization on each server after rebalance

These checks warn anyway regardless of lowDiskMode is set or not, where the lowDiskMode config is designed to solve the case when the first fails and the second passes.

New Behavior

Make it only one diskUtilization check. Pass when both the original diskUtilizationDuringRebalance and diskUtilizationAfterRebalance passes, or only diskUtilizationAfterRebalance passes yet lowDiskMode=true, downtime=false, fail otherwise.

Examples

1. Within threshold throughout

"diskUtilization": {
  "preCheckStatus": "PASS",
  "message": "Within threshold (<90%)"
}

2. Over threshold after the rebalance — ERROR regardless of config

"diskUtilization": {
  "preCheckStatus": "ERROR",
  "message": "UNSAFE. Servers with unsafe disk utilization AFTER rebalance (>90%): Server_localhost_8099 (92%)"
}

3. Over threshold only during the rebalance, lowDiskMode=false → ERROR, with the fix named

"diskUtilization": {
  "preCheckStatus": "ERROR",
  "message": "UNSAFE. Servers with unsafe disk utilization DURING rebalance (>90%): Server_localhost_8098 (93%). Enable lowDiskMode to delete segments before adding the new ones"
}

4. Same, lowDiskMode=true → PASS, transient peak is ruled out

"diskUtilization": {
  "preCheckStatus": "PASS",
  "message": "Within threshold (<90%) after rebalance. Some servers would go over it during the rebalance, but lowDiskMode avoids that transient disk usage"
}

5. Same, lowDiskMode=true AND downtime=true → still ERROR, because downtime replaces the IdealState in one go and skips the incremental path lowDiskMode acts on

"diskUtilization": {
  "preCheckStatus": "ERROR",
  "message": "UNSAFE. Servers with unsafe disk utilization during rebalance (>90%): Server_localhost_8098 (93%). lowDiskMode has no effect while downtime is enabled, disable downtime for it to delete segments before adding the new ones"
}

6. Disk usage info not yet collected — unchanged

"diskUtilization": {
  "preCheckStatus": "WARN",
  "message": "Disk usage info has not been updated. Try later or set controller.resource.utilization.checker.initial.delay to a shorter period"
}

7. rebalanceConfigOptions also flags the dead combination, even when disk is fine

"rebalanceConfigOptions": {
  "preCheckStatus": "WARN",
  "message": "lowDiskMode has no effect when downtime is enabled, disable downtime for segments to be deleted before the new ones are added."
}

codecov-commenter · 2026-08-07T00:56:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.62%. Comparing base (504efc4) to head (a25d60a).
⚠️ Report is 18 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #19176      +/-   ##
============================================
+ Coverage     65.70%   66.62%   +0.91%     
  Complexity     1423     1423              
============================================
  Files          3439     3443       +4     
  Lines        218064   218584     +520     
  Branches      34679    34793     +114     
============================================
+ Hits         143289   145624    +2335     
+ Misses        63226    61231    -1995     
- Partials      11549    11729     +180

Flag	Coverage Δ
custom-integration1	`100.00% <ø> (ø)`
integration	`100.00% <ø> (ø)`
integration1	`100.00% <ø> (ø)`
integration2	`0.00% <ø> (ø)`
java-25	`66.62% <100.00%> (+0.91%)`	⬆️
temurin	`66.62% <100.00%> (+0.91%)`	⬆️
unittests	`66.61% <100.00%> (+0.91%)`	⬆️
unittests1	`57.13% <ø> (+0.09%)`	⬆️
unittests2	`38.93% <100.00%> (+0.91%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

yashmayya

Thanks for the improvement, I've left some comments

yashmayya · 2026-08-07T05:04:28Z

+      addIfOverThreshold(serversUnsafeDuringRebalance, server,
+          (double) (diskUsage.getUsedSpaceBytes() + diskUtilizationGain) / diskUsage.getTotalSpaceBytes(), threshold);


We'll be flagging servers where the existing used space is over the threshold even if diskUtilizationGain == 0? Seems wrong to say unsafe DURING rebalance / recommend enabling lowDiskMode which would do nothing in such a scenario.

yashmayya · 2026-08-07T05:05:12Z

+    // deleted before adding the new ones. It is however only honored by the incremental rebalance path, which downtime
+    // skips altogether by replacing the IdealState with the target assignment in one go
+    RebalanceConfig rebalanceConfig = preCheckContext.getRebalanceConfig();
+    if (rebalanceConfig.isLowDiskMode() && !rebalanceConfig.isDowntime()) {


Should we also check for bestEfforts which seems to also potentially violate lowDiskMode?

Afaik bestEfforts is orthogonal to lowDiskMode, can you explain your concern?

If the EV-IS progress stalls, IIUC bestEfforts will cause the rebalance algorithm to give up waiting and just move to the next step which could mean that we add new segments to servers before old ones are actually offloaded?

yashmayya · 2026-08-07T05:06:05Z

+    // skips altogether by replacing the IdealState with the target assignment in one go
+    RebalanceConfig rebalanceConfig = preCheckContext.getRebalanceConfig();
+    if (rebalanceConfig.isLowDiskMode() && !rebalanceConfig.isDowntime()) {
+      return RebalancePreCheckerResult.pass(withinThreshold + " AFTER rebalance. Some servers would go over it DURING "


Might be useful to log which servers

yashmayya · 2026-08-07T05:06:58Z

+  }
+
+  private static String getUnsafeDiskUtilizationMessage(String when, List<String> servers, double threshold) {
+    return String.format("UNSAFE. Servers with unsafe disk utilization %s (>%d%%): %s", when, (short) (threshold * 100),


>

Should it be >=?

J-HowHuang added 2 commits August 6, 2026 14:57

consolidate two disk util rebalance prechecks into one

1b7fd8c

patch downtime logic

2de0216

J-HowHuang added observability Related to observability (logging, tracing, metrics) segment-rebalance Related to segment rebalancing across servers labels Aug 6, 2026

capitalize keywords

a25d60a

J-HowHuang requested a review from yashmayya August 7, 2026 03:59

yashmayya reviewed Aug 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

Consolidate two disk util rebalance prechecks into one - #19176

Consolidate two disk util rebalance prechecks into one#19176
J-HowHuang wants to merge 3 commits into
apache:masterfrom
J-HowHuang:consolidate-disk-util-precheck

J-HowHuang commented Aug 6, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Aug 7, 2026 •

edited

Loading

Uh oh!

yashmayya left a comment

Uh oh!

yashmayya Aug 7, 2026

Uh oh!

yashmayya Aug 7, 2026

Uh oh!

J-HowHuang Aug 7, 2026

Uh oh!

yashmayya Aug 7, 2026

Uh oh!

yashmayya Aug 7, 2026

Uh oh!

yashmayya Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		addIfOverThreshold(serversUnsafeDuringRebalance, server,
		(double) (diskUsage.getUsedSpaceBytes() + diskUtilizationGain) / diskUsage.getTotalSpaceBytes(), threshold);

Uh oh!

Conversation

J-HowHuang commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Original Behavior

New Behavior

Examples

Uh oh!

codecov-commenter commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yashmayya left a comment

Choose a reason for hiding this comment

Uh oh!

yashmayya Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

yashmayya Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

J-HowHuang Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

yashmayya Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

yashmayya Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

yashmayya Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

J-HowHuang commented Aug 6, 2026 •

edited

Loading

codecov-commenter commented Aug 7, 2026 •

edited

Loading