Fail reload if derived columns can't be created (#11559)

apache · Sep 12, 2023 · bc0d9fb · bc0d9fb
1 parent b297245
commit bc0d9fb
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 2 deletions.
diff --git a/...src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java b/...src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java
@@ -104,6 +104,7 @@ public class IndexLoadingConfig {
   private boolean _isDirectRealtimeOffHeapAllocation;
   private boolean _enableSplitCommitEndWithMetadata;
   private String _segmentStoreURI;
+  private boolean _errorOnColumnBuildFailure;
 
   // constructed from FieldConfig
   private Map<String, Map<String, String>> _columnProperties = new HashMap<>();
@@ -880,6 +881,14 @@ public void setTableDataDir(String tableDataDir) {
     _dirty = true;
   }
 
+  public boolean isErrorOnColumnBuildFailure() {
+    return _errorOnColumnBuildFailure;
+  }
+
+  public void setErrorOnColumnBuildFailure(boolean errorOnColumnBuildFailure) {
+    _errorOnColumnBuildFailure = errorOnColumnBuildFailure;
+  }
+
   public String getTableDataDir() {
     return _tableDataDir;
   }

diff --git a/...ache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java b/...ache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
@@ -347,6 +347,7 @@ protected void removeColumnIndices(String column)
   protected boolean createColumnV1Indices(String column)
       throws Exception {
     TableConfig tableConfig = _indexLoadingConfig.getTableConfig();
+    boolean errorOnFailure = _indexLoadingConfig.isErrorOnColumnBuildFailure();
     if (tableConfig != null && tableConfig.getIngestionConfig() != null
         && tableConfig.getIngestionConfig().getTransformConfigs() != null) {
       List<TransformConfig> transformConfigs = tableConfig.getIngestionConfig().getTransformConfigs();
@@ -364,6 +365,11 @@ protected boolean createColumnV1Indices(String column)
             if (columnMetadata == null) {
               LOGGER.warn("Skip creating derived column: {} because argument: {} does not exist in the segment", column,
                   argument);
+              if (errorOnFailure) {
+                throw new RuntimeException(String.format(
+                    "Failed to create derived column: %s because argument: %s does not exist in the segment", column,
+                    argument));
+              }
               return false;
             }
             // TODO: Support creation of derived columns from forward index disabled columns
@@ -379,12 +385,19 @@ protected boolean createColumnV1Indices(String column)
           // TODO: Support raw derived column
           if (_indexLoadingConfig.getNoDictionaryColumns().contains(column)) {
             LOGGER.warn("Skip creating raw derived column: {}", column);
+            if (errorOnFailure) {
+              throw new UnsupportedOperationException(String.format("Failed to create raw derived column: %s", column));
+            }
             return false;
           }
 
           // TODO: Support forward index disabled derived column
           if (_indexLoadingConfig.getForwardIndexDisabledColumns().contains(column)) {
             LOGGER.warn("Skip creating forward index disabled derived column: {}", column);
+            if (errorOnFailure) {
+              throw new UnsupportedOperationException(
+                  String.format("Failed to create forward index disabled derived column: %s", column));
+            }
             return false;
           }
 
@@ -394,6 +407,9 @@ protected boolean createColumnV1Indices(String column)
           } catch (Exception e) {
             LOGGER.error("Caught exception while creating derived column: {} with transform function: {}", column,
                 transformFunction, e);
+            if (errorOnFailure) {
+              throw e;
+            }
             return false;
           }
         }

diff --git a/...-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java b/...-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java
@@ -472,8 +472,9 @@ private void reloadSegmentWithMetadata(String tableNameWithType, SegmentMetadata
       segmentLock.lock();
 
       // Reloads an existing segment, and the local segment metadata is existing as asserted above.
-      tableDataManager.reloadSegment(segmentName,
-          new IndexLoadingConfig(_instanceDataManagerConfig, tableConfig, schema), zkMetadata, segmentMetadata, schema,
+      IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(_instanceDataManagerConfig, tableConfig, schema);
+      indexLoadingConfig.setErrorOnColumnBuildFailure(true);
+      tableDataManager.reloadSegment(segmentName, indexLoadingConfig, zkMetadata, segmentMetadata, schema,
           forceDownload);
       LOGGER.info("Reloaded segment: {} of table: {}", segmentName, tableNameWithType);
     } finally {