xiangfu0 commented on code in PR #11782:
URL: https://github.com/apache/pinot/pull/11782#discussion_r1357876369
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -1559,33 +1560,32 @@ public void addTable(TableConfig tableConfig)
if (ZKMetadataProvider.getSchema(_propertyStore,
TableNameBuilder.extractRawTableName(tableNameWithType)) == null) {
throw new InvalidTableConfigException("No schema defined for table: " +
tableNameWithType);
}
- if (tableType == TableType.OFFLINE) {
- try {
- // Add table config
- ZKMetadataProvider.setTableConfig(_propertyStore, tableNameWithType,
TableConfigUtils.toZNRecord(tableConfig));
- // Assign instances
- assignInstances(tableConfig, true);
+ Preconditions.checkState(tableType == TableType.OFFLINE || tableType ==
TableType.REALTIME,
+ "Invalid table type: %s", tableType);
+ try {
+ // Add table config
+ if (!ZKMetadataProvider.createTableConfig(_propertyStore,
tableNameWithType, tableConfig)) {
+ if (getTableConfig(tableNameWithType) != null) {
+ throw new TableAlreadyExistsException("Table config for " +
tableNameWithType
+ + " already exists. If this is unexpected, try deleting the
table to remove all metadata associated"
+ + " with it.");
+ } else {
+ throw new RuntimeException("Failed to create table config for table:
" + tableNameWithType);
+ }
+ }
+ // Assign instances
+ assignInstances(tableConfig, true);
+ if (tableType == TableType.OFFLINE) {
// Add ideal state
_helixAdmin.addResource(_helixClusterName, tableNameWithType,
idealState);
- } catch (Exception e) {
- LOGGER.error("Caught exception during offline table setup. Cleaning up
table {}", tableNameWithType, e);
- deleteOfflineTable(tableNameWithType);
- throw e;
- }
- } else {
- Preconditions.checkState(tableType == TableType.REALTIME, "Invalid table
type: %s", tableType);
- try {
- // Add table config
- ZKMetadataProvider.setTableConfig(_propertyStore, tableNameWithType,
TableConfigUtils.toZNRecord(tableConfig));
- // Assign instances
- assignInstances(tableConfig, true);
+ } else {
// Add ideal state with the first CONSUMING segment
_pinotLLCRealtimeSegmentManager.setUpNewTable(tableConfig, idealState);
- } catch (Exception e) {
- LOGGER.error("Caught exception during realtime table setup. Cleaning
up table {}", tableNameWithType, e);
- deleteRealtimeTable(tableNameWithType);
- throw e;
}
+ } catch (Exception e) {
+ LOGGER.error("Caught exception during offline table setup. Cleaning up
table {}", tableNameWithType, e);
+ deleteTable(tableNameWithType, tableType, null);
Review Comment:
Moved it.
I think technically we should have a cluster level lock to ensure certain
options won't take place simultaneously.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
