From bc0d9fb65916bb87fa5272e94007a94a7ffdc569 Mon Sep 17 00:00:00 2001 From: Saurabh Dubey Date: Tue, 12 Sep 2023 11:00:24 +0530 Subject: [PATCH] Fail reload if derived columns can't be created (#11559) --- .../segment/index/loader/IndexLoadingConfig.java | 9 +++++++++ .../defaultcolumn/BaseDefaultColumnHandler.java | 16 ++++++++++++++++ .../starter/helix/HelixInstanceDataManager.java | 5 +++-- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java index 66c0e3b6c4b2..e18efd2900ba 100644 --- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java +++ b/pinot-segment-local/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> _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/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java index f2148dce9ed4..a8c6f91917d7 100644 --- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java +++ b/pinot-segment-local/src/main/java/org/apache/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 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/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java index d5c0c8afd841..9fd9c3e44af0 100644 --- a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java +++ b/pinot-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 {