You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/12/06 23:40:16 UTC
[GitHub] [pinot] navina commented on a diff in pull request #9920: Clean up TableDataManagerConfig to be a thin wrapper over instance and table config
navina commented on code in PR #9920:
URL: https://github.com/apache/pinot/pull/9920#discussion_r1041585702
##########
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:
Is the auth config same for the server and the table data manager ?
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -324,6 +324,11 @@ public File getTableDataDir() {
return _indexDir;
}
+ @Override
+ public TableDataManagerConfig getTableDataManagerConfig() {
Review Comment:
I am guessing this change make this PR https://github.com/apache/pinot/pull/9918/files moot?
+1 to `TableDataManager#getTableDataManagerConfig`
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org