swaminathanmanish commented on code in PR #15008:
URL: https://github.com/apache/pinot/pull/15008#discussion_r1954759852


##########
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java:
##########
@@ -320,6 +328,12 @@ private static long getRandomInitialDelayInSeconds() {
   private static final int DEFAULT_MIN_NUM_CHARS_IN_IS_TO_TURN_ON_COMPRESSION 
= -1;
   private static final int DEFAULT_REALTIME_SEGMENT_METADATA_COMMIT_NUMLOCKS = 
64;
   private static final boolean DEFAULT_ENABLE_STORAGE_QUOTA_CHECK = true;
+  private static final double DEFAULT_DISK_UTILIZATION_THRESHOLD = 0.95f;

Review Comment:
   I think this can be very close to full space utilization ( >= 99%). Stopping 
ingestion should be the last resort. 



##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerGauge.java:
##########
@@ -187,7 +187,10 @@ public enum ControllerGauge implements 
AbstractMetrics.Gauge {
   TABLE_REBALANCE_IN_PROGRESS("tableRebalanceInProgress", false),
 
   // Number of reingested segments getting uploaded
-  REINGESTED_SEGMENT_UPLOADS_IN_PROGRESS("reingestedSegmentUploadsInProgress", 
true);
+  REINGESTED_SEGMENT_UPLOADS_IN_PROGRESS("reingestedSegmentUploadsInProgress", 
true),
+
+  // Resource utilization is within limits or not for a table
+  RESOURCE_UTILIZATION_LIMIT_EXCEEDED("ResourceUtilizationLimitExceeded", 
false);

Review Comment:
   As follow up, it'll be good to add individual metrics and alerts for a 
resource (eg: disk)



##########
pinot-controller/src/main/java/org/apache/pinot/controller/validation/DiskUtilizationChecker.java:
##########
@@ -0,0 +1,134 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.validation;
+
+import com.google.common.collect.BiMap;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.pinot.common.restlet.resources.DiskUsageInfo;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.apache.pinot.controller.util.CompletionServiceHelper;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+public class DiskUtilizationChecker {
+  private static final Logger LOGGER = 
LoggerFactory.getLogger(DiskUtilizationChecker.class);
+  private final int _resourceUtilizationCheckTimeoutMs;
+  private final long _resourceUtilizationCheckerFrequency;
+  private final double _diskUtilizationThreshold;
+  private final String _diskUtilizationPath;
+  private static final String DISK_UTILIZATION_API_PATH = 
"/instance/diskUtilization";
+
+  private final PinotHelixResourceManager _helixResourceManager;
+
+  public DiskUtilizationChecker(PinotHelixResourceManager 
helixResourceManager, ControllerConf controllerConf) {
+    _helixResourceManager = helixResourceManager;
+    _diskUtilizationPath = controllerConf.getDiskUtilizationPath();
+    _diskUtilizationThreshold = controllerConf.getDiskUtilizationThreshold();
+    _resourceUtilizationCheckTimeoutMs = 
controllerConf.getDiskUtilizationCheckTimeoutMs();
+    _resourceUtilizationCheckerFrequency = 
controllerConf.getResourceUtilizationCheckerFrequency();
+  }
+
+  public static String getDiskUtilizationApiPath() {
+    return DISK_UTILIZATION_API_PATH;
+  }
+
+  /**
+   * Check if disk utilization for the requested table is within the 
configured limits.
+   */
+  public boolean isDiskUtilizationWithinLimits(String tableNameWithType) {
+    if (StringUtils.isEmpty(tableNameWithType)) {
+      throw new IllegalArgumentException("Table name found to be null or empty 
while computing disk utilization.");
+    }
+    TableConfig tableConfig = 
_helixResourceManager.getTableConfig(tableNameWithType);
+    if (tableConfig == null) {
+      return true; // table does not exist
+    }
+    List<String> instances;
+    if (TableNameBuilder.isOfflineTableResource(tableNameWithType)) {
+      instances = 
_helixResourceManager.getServerInstancesForTable(tableNameWithType, 
TableType.OFFLINE);
+    } else {
+      instances = 
_helixResourceManager.getServerInstancesForTable(tableNameWithType, 
TableType.REALTIME);
+    }
+    return isDiskUtilizationWithinLimits(instances);
+  }
+
+  private boolean isDiskUtilizationWithinLimits(List<String> instances) {
+    for (String instance : instances) {
+      DiskUsageInfo diskUsageInfo = 
ResourceUtilizationInfo.getDiskUsageInfo(instance);
+      if (diskUsageInfo == null) {
+        LOGGER.warn("Disk utilization info for server: {} is null", instance);
+        continue;
+      }
+      // Ignore if the disk utilization info is stale. The info is considered 
stale if it is older than the
+      // ResourceUtilizationChecker tasks frequency.
+      if (diskUsageInfo.getLastUpdatedTimeInEpochMs()

Review Comment:
   Ok this is like the safeguard mechanism to not block ingestion if stats are 
not updated. 



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

Reply via email to