xiangfu0 commented on code in PR #10027: URL: https://github.com/apache/pinot/pull/10027#discussion_r1061933883
########## pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/cleanup/StaleInstancesCleanupTask.java: ########## @@ -82,7 +95,47 @@ protected void runTask(Properties periodicTaskProperties) { _controllerMetrics.addValueToGlobalGauge(ControllerGauge.DROPPED_MINION_INSTANCES, 1); } } + continue; + } + + // Drop the broker instance if it has been offline for more than a period of this task. + if (InstanceTypeUtils.isBroker(offlineInstance) && !brokerInstancesInUse.contains(offlineInstance)) { + if (_pinotHelixResourceManager.isInstanceOfflineFor(offlineInstance, + _staleInstancesCleanupTaskMinOfflineTimeBeforeDeletionInMilliseconds)) { + LOGGER.info("Dropping broker instance: {}", offlineInstance); + PinotResourceManagerResponse response = _pinotHelixResourceManager.dropInstance(offlineInstance); + if (response.isSuccessful()) { + _controllerMetrics.addValueToGlobalGauge(ControllerGauge.DROPPED_BROKER_INSTANCES, 1); + } + } + continue; + } + + // Drop the server instance if it has been offline for more than a period of this task. + if (InstanceTypeUtils.isServer(offlineInstance) && !serverInstancesInUse.contains(offlineInstance)) { + if (_pinotHelixResourceManager.isInstanceOfflineFor(offlineInstance, + _staleInstancesCleanupTaskMinOfflineTimeBeforeDeletionInMilliseconds)) { + LOGGER.info("Dropping server instance: {}", offlineInstance); + PinotResourceManagerResponse response = _pinotHelixResourceManager.dropInstance(offlineInstance); + if (response.isSuccessful()) { + _controllerMetrics.addValueToGlobalGauge(ControllerGauge.DROPPED_SERVER_INSTANCES, 1); + } + } } } } + + private Set<String> getBrokerInstancesInUse() { Review Comment: updated to use brokerResource ########## pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java: ########## @@ -139,6 +139,14 @@ public static class ControllerPeriodicTasksConf { "controller.minion.instances.cleanup.task.minOfflineTimeBeforeDeletionSeconds"; public static final String MINION_INSTANCES_CLEANUP_TASK_MIN_OFFLINE_TIME_BEFORE_DELETION_PERIOD = "controller.minion.instances.cleanup.task.minOfflineTimeBeforeDeletionPeriod"; + + public static final String STALE_INSTANCES_CLEANUP_TASK_FREQUENCY_PERIOD = Review Comment: done ########## pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerGauge.java: ########## @@ -99,9 +99,16 @@ public enum ControllerGauge implements AbstractMetrics.Gauge { // Number of Tasks Status TASK_STATUS("taskStatus", false), - // Number of dropped minion instances + // Number of dropped stale minion instances DROPPED_MINION_INSTANCES("droppedMinionInstances", true), + // Number of dropped stale broker instances + DROPPED_BROKER_INSTANCES("droppedBrokerInstances", true), + + // Number of dropped stale server instances + DROPPED_SERVER_INSTANCES("droppedServerInstances", true), + + Review Comment: done -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org