shounakmk219 opened a new pull request, #18776:
URL: https://github.com/apache/pinot/pull/18776

   ## Description
   
   `instanceDropSafetyCheck` (called by `dropInstance` and the 
`/instances/dropInstance/validate` endpoint) verifies an instance hosts no 
resource by scanning **every** resource's `IdealState` across the cluster. For 
minion instances this scan is pure waste:
   
   - `getAllResources()` returns only table (`_OFFLINE`/`_REALTIME`) and broker 
resources.
   - Minions never appear in any of those IdealStates — minion task assignments 
are managed by the Helix Task Framework in `JobContext`/`WorkflowContext`, not 
in IdealState.
   
   So the scan can only ever return empty for a minion, yet each minion drop 
pulls every IdealState into the controller heap. When many minions are dropped 
in succession this creates significant heap pressure / GC churn on the 
controller.
   
   ## Change
   
   - Gate the resource scan on `!InstanceTypeUtils.isMinion(instanceName)`.
   - Keep the cheap `IS_ALIVE` liveness check (a single ZK read) for **all** 
instance types — a live minion is still reported as unsafe to drop.
   - The scan remains in place for controllers/servers/brokers. This is 
deliberate: controllers are real participants in the `leadControllerResource` 
Helix resource and can legitimately match `CONTAINS_RESOURCE`, so the skip is 
scoped to minions only — not a broader "non-server" condition.
   
   ## Tests
   
   Added to `PinotHelixResourceManagerMinionDrainTest`:
   - `testInstanceDropSafetyCheckSkipsResourceScanForMinion` — spies the 
resource manager and verifies `getAllResources()` is **never** invoked for a 
minion (proves the scan is bypassed, not just result-equivalent).
   - `testInstanceDropSafetyCheckRunsResourceScanForNonMinion` — verifies the 
scan **still runs** for a broker, locking the `!isMinion` guard against 
inversion.
   - `testInstanceDropSafetyCheckFlagsLiveMinion` — verifies a live minion is 
still flagged `IS_ALIVE`.
   
   All 15 tests in the class pass; spotless/checkstyle/license checks pass on 
`pinot-controller`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to