Jackie-Jiang commented on code in PR #9920:
URL: https://github.com/apache/pinot/pull/9920#discussion_r1041587929


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/data/manager/TableDataManagerConfig.java:
##########
@@ -18,119 +18,75 @@
  */
 package org.apache.pinot.segment.local.data.manager;
 
-import com.google.common.base.Preconditions;
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.PropertiesConfiguration;
 import org.apache.pinot.spi.config.instance.InstanceDataManagerConfig;
-import org.apache.pinot.spi.config.table.SegmentsValidationAndRetentionConfig;
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.config.table.TableType;
-import org.apache.pinot.spi.utils.builder.TableNameBuilder;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 
 /**
  * The config used for TableDataManager.
  */
 public class TableDataManagerConfig {
-  private static final Logger LOGGER = 
LoggerFactory.getLogger(TableDataManagerConfig.class);
-
-  private static final String TABLE_DATA_MANAGER_TYPE = "dataManagerType";
-  private static final String TABLE_DATA_MANAGER_DATA_DIRECTORY = "directory";
-  private static final String TABLE_DATA_MANAGER_CONSUMER_DIRECTORY = 
"consumerDirectory";
-  private static final String TABLE_DATA_MANAGER_NAME = "name";
-  private static final String TABLE_IS_DIMENSION = "isDimTable";
-  private static final String TABLE_DATA_MANAGER_AUTH = "auth";
-  private static final String TABLE_DELETED_SEGMENTS_CACHE_SIZE = 
"deletedSegmentsCacheSize";
-  private static final String TABLE_DELETED_SEGMENTS_CACHE_TTL_MINUTES = 
"deletedSegmentsCacheTTL";
-  private static final String TABLE_PEER_DOWNLOAD_SCHEME = 
"peerDownloadScheme";
-
-  private final Configuration _tableDataManagerConfig;
-
-  public TableDataManagerConfig(Configuration tableDataManagerConfig) {
-    _tableDataManagerConfig = tableDataManagerConfig;
-  }
+  public static final String AUTH_CONFIG_PREFIX = "auth";
 
-  public Configuration getConfig() {
-    return _tableDataManagerConfig;
-  }
+  private final InstanceDataManagerConfig _instanceDataManagerConfig;
+  private final TableConfig _tableConfig;
 
-  public String getTableDataManagerType() {
-    return _tableDataManagerConfig.getString(TABLE_DATA_MANAGER_TYPE);
+  public TableDataManagerConfig(InstanceDataManagerConfig 
instanceDataManagerConfig, TableConfig tableConfig) {
+    _instanceDataManagerConfig = instanceDataManagerConfig;
+    _tableConfig = tableConfig;
   }
 
-  public String getDataDir() {
-    return 
_tableDataManagerConfig.getString(TABLE_DATA_MANAGER_DATA_DIRECTORY);
+  public InstanceDataManagerConfig getInstanceDataManagerConfig() {
+    return _instanceDataManagerConfig;
   }
 
-  public String getConsumerDir() {
-    return 
_tableDataManagerConfig.getString(TABLE_DATA_MANAGER_CONSUMER_DIRECTORY);
+  public TableConfig getTableConfig() {
+    return _tableConfig;
   }
 
   public String getTableName() {
-    return _tableDataManagerConfig.getString(TABLE_DATA_MANAGER_NAME);
+    return _tableConfig.getTableName();
   }
 
-  public boolean isDimTable() {
-    return _tableDataManagerConfig.getBoolean(TABLE_IS_DIMENSION);
+  public TableType getTableType() {
+    return _tableConfig.getTableType();
   }
 
-  public Configuration getAuthConfig() {
-    return _tableDataManagerConfig.subset(TABLE_DATA_MANAGER_AUTH);
+  public boolean isDimTable() {
+    return _tableConfig.isDimTable();
   }
 
-  public int getTableDeletedSegmentsCacheSize() {
-    return _tableDataManagerConfig.getInt(TABLE_DELETED_SEGMENTS_CACHE_SIZE);
+  public String getDataDir() {
+    return _instanceDataManagerConfig.getInstanceDataDir() + "/" + 
getTableName();
   }
 
-  public int getTableDeletedSegmentsCacheTtlMinutes() {
-    return 
_tableDataManagerConfig.getInt(TABLE_DELETED_SEGMENTS_CACHE_TTL_MINUTES);
+  public String getConsumerDir() {
+    return _instanceDataManagerConfig.getConsumerDir();
   }
 
   public String getTablePeerDownloadScheme() {
-    return _tableDataManagerConfig.getString(TABLE_PEER_DOWNLOAD_SCHEME);
+    String peerSegmentDownloadScheme = 
_tableConfig.getValidationConfig().getPeerSegmentDownloadScheme();
+    if (peerSegmentDownloadScheme != null) {
+      return peerSegmentDownloadScheme;
+    }
+    return _instanceDataManagerConfig.getSegmentPeerDownloadScheme();
   }
 
-  public static TableDataManagerConfig getDefaultHelixTableDataManagerConfig(
-      InstanceDataManagerConfig instanceDataManagerConfig, String 
tableNameWithType) {
-    Configuration defaultConfig = new PropertiesConfiguration();
-    defaultConfig.addProperty(TABLE_DATA_MANAGER_NAME, tableNameWithType);
-    defaultConfig.addProperty(TABLE_DATA_MANAGER_DATA_DIRECTORY,
-        instanceDataManagerConfig.getInstanceDataDir() + "/" + 
tableNameWithType);
-    defaultConfig.addProperty(TABLE_DATA_MANAGER_CONSUMER_DIRECTORY, 
instanceDataManagerConfig.getConsumerDir());
-    TableType tableType = 
TableNameBuilder.getTableTypeFromTableName(tableNameWithType);
-    Preconditions.checkNotNull(tableType);
-    defaultConfig.addProperty(TABLE_DATA_MANAGER_TYPE, tableType.name());
-    defaultConfig.addProperty(TABLE_DELETED_SEGMENTS_CACHE_SIZE,
-        instanceDataManagerConfig.getDeletedSegmentsCacheSize());
-    defaultConfig.addProperty(TABLE_DELETED_SEGMENTS_CACHE_TTL_MINUTES,
-        instanceDataManagerConfig.getDeletedSegmentsCacheTtlMinutes());
-    // allow null
-    String segmentPeerDownloadScheme = 
instanceDataManagerConfig.getSegmentPeerDownloadScheme();
-    LOGGER.info("instance level segment peer download scheme = {}", 
segmentPeerDownloadScheme);
-    defaultConfig.addProperty(TABLE_PEER_DOWNLOAD_SCHEME, 
segmentPeerDownloadScheme);
-
-
-    // copy auth-related configs
-    
instanceDataManagerConfig.getConfig().subset(TABLE_DATA_MANAGER_AUTH).toMap()
-        .forEach((key, value) -> 
defaultConfig.setProperty(TABLE_DATA_MANAGER_AUTH + "." + key, value));
-
-    return new TableDataManagerConfig(defaultConfig);
+  public int getTableDeletedSegmentsCacheSize() {
+    return _instanceDataManagerConfig.getDeletedSegmentsCacheSize();
   }
 
-  public void overrideConfigs(TableConfig tableConfig) {
-    // Override table level configs
-
-    _tableDataManagerConfig.addProperty(TABLE_IS_DIMENSION, 
tableConfig.isDimTable());
-    SegmentsValidationAndRetentionConfig segmentConfig = 
tableConfig.getValidationConfig();
-    if (segmentConfig != null && segmentConfig.getPeerSegmentDownloadScheme() 
!= null) {
-      _tableDataManagerConfig.setProperty(TABLE_PEER_DOWNLOAD_SCHEME, 
segmentConfig.getPeerSegmentDownloadScheme());
-    }
-    LOGGER.info("final segment peer download scheme = {}", 
getTablePeerDownloadScheme());
+  public int getTableDeletedSegmentsCacheTtlMinutes() {
+    return _instanceDataManagerConfig.getDeletedSegmentsCacheTtlMinutes();
+  }
 
-    // If we wish to override some table level configs using table config, 
override them here
-    // Note: the configs in TableDataManagerConfig is immutable once the table 
is created, which mean it will not pick
-    // up the latest table config
+  public Configuration getAuthConfig() {

Review Comment:
   Based on the current implementation, it is the same. I assume we will need 
table level auth in the future



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

Reply via email to