Jackie-Jiang commented on code in PR #11574: URL: https://github.com/apache/pinot/pull/11574#discussion_r1353040724
########## pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java: ########## @@ -549,6 +556,83 @@ protected void configure() { _serviceStatusCallbackList.add(generateServiceStatusCallback(_helixParticipantManager)); } + /** + * This method is used to fix table/schema names. + * TODO: in the next release, maybe 2.0.0, we can remove this method. Meanwhile we can delete the orphan schemas + * that has been existed longer than a certain time period. + * + */ + @VisibleForTesting + public void fixSchemaNameInTableConfig() { + AtomicInteger fixedTableCount = new AtomicInteger(); + AtomicInteger failedToFixTableCount = new AtomicInteger(); + AtomicInteger misConfiguredTableCount = new AtomicInteger(); + List<String> allTables = _helixResourceManager.getAllTables(); + + allTables.forEach(table -> { + TableConfig tableConfig = _helixResourceManager.getTableConfig(table); + if ((tableConfig == null) || (tableConfig.getValidationConfig() == null)) { + LOGGER.warn("Failed to find table config for table: {}", table); + failedToFixTableCount.getAndIncrement(); + return; + } + String rawTableName = TableNameBuilder.extractRawTableName(tableConfig.getTableName()); + String existSchemaName = tableConfig.getValidationConfig().getSchemaName(); + if (existSchemaName == null) { + // Although the table config is valid, we still need to ensure the schema exists + if (_helixResourceManager.getSchema(rawTableName) == null) { + LOGGER.warn("Failed to find schema for table: {}", rawTableName); + failedToFixTableCount.getAndIncrement(); + return; + } + // Table config is already in good status + return; + } + if (_helixResourceManager.getSchema(rawTableName) != null) { + // If a schema named `rawTableName` already exists, then likely this is a misconfiguration. + // Reset schema name in table config to null to let the table point to the existing schema. + LOGGER.warn("Schema: {} already exists, fix the schema name in table config from {} to null", rawTableName, + existSchemaName); + misConfiguredTableCount.getAndIncrement(); + } else { + // Copy the schema current table referring to to `rawTableName` if it does not exist + Schema schema = _helixResourceManager.getSchema(existSchemaName); + if (schema == null) { + LOGGER.warn("Failed to find schema for schema name: {}", existSchemaName); + failedToFixTableCount.getAndIncrement(); + return; + } + schema.setSchemaName(rawTableName); + try { + _helixResourceManager.addSchema(schema, false, false); + LOGGER.info("Copied schema: {} to {}", existSchemaName, rawTableName); + } catch (Exception e) { + LOGGER.error("Failed to copy schema: {} to {}", existSchemaName, rawTableName, e); + failedToFixTableCount.getAndIncrement(); + return; + } + } + // Update table config to remove schema name + tableConfig.getValidationConfig().setSchemaName(null); + try { + _helixResourceManager.updateTableConfig(tableConfig); Review Comment: We don't need to change `PinotHelixResouceManager`. Actually we should not even use this `updateTableConfig()` method, but should use property store instead which can check the versions. `PinotHelixResourceManager.updateTableConfig()` is always overriding the whole table config, instead of changing a field, and that is why version check is not required -- 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