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 22:21:28 UTC
[GitHub] [pinot] Jackie-Jiang opened a new pull request, #9920: Clean up TableDataManagerConfig to be a thin wrapper over instance and table config
Jackie-Jiang opened a new pull request, #9920:
URL: https://github.com/apache/pinot/pull/9920
Clean up `TableDataManagerConfig` to directly access the instance and table config, and avoid the unnecessary creation of a separate map. Also fix some wrong usages of raw table name in the tests
--
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
[GitHub] [pinot] navina commented on a diff in pull request #9920: Clean up TableDataManagerConfig to be a thin wrapper over instance and table config
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [pinot] KKcorps commented on a diff in pull request #9920: Clean up TableDataManagerConfig to be a thin wrapper over instance and table config
Posted by GitBox <gi...@apache.org>.
KKcorps commented on code in PR #9920:
URL: https://github.com/apache/pinot/pull/9920#discussion_r1043015864
##########
pinot-core/src/test/java/org/apache/pinot/queries/ExplainPlanQueriesTest.java:
##########
@@ -284,7 +285,7 @@ public void setUp()
tableDataManager.addSegment((ImmutableSegment) indexSegment);
}
InstanceDataManager instanceDataManager = mock(InstanceDataManager.class);
- when(instanceDataManager.getTableDataManager(RAW_TABLE_NAME)).thenReturn(tableDataManager);
+ when(instanceDataManager.getTableDataManager(OFFLINE_TABLE_NAME)).thenReturn(tableDataManager);
Review Comment:
This change is breaking the test. QueryExecutor is getting table name without type as argument and hence gets no tableDataManager
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9920: Clean up TableDataManagerConfig to be a thin wrapper over instance and table config
Posted by GitBox <gi...@apache.org>.
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: 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
[GitHub] [pinot] Jackie-Jiang merged pull request #9920: Clean up TableDataManagerConfig to be a thin wrapper over instance and table config
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged PR #9920:
URL: https://github.com/apache/pinot/pull/9920
--
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
[GitHub] [pinot] KKcorps commented on pull request #9920: Clean up TableDataManagerConfig to be a thin wrapper over instance and table config
Posted by GitBox <gi...@apache.org>.
KKcorps commented on PR #9920:
URL: https://github.com/apache/pinot/pull/9920#issuecomment-1341601312
@Jackie-Jiang merge conflicts in this and some tests failing as well. Seems like some issue with tableNameWithType in ExplainPlanQueriesTest.
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9920: Clean up TableDataManagerConfig to be a thin wrapper over instance and table config
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9920:
URL: https://github.com/apache/pinot/pull/9920#discussion_r1043668219
##########
pinot-core/src/test/java/org/apache/pinot/queries/ExplainPlanQueriesTest.java:
##########
@@ -284,7 +285,7 @@ public void setUp()
tableDataManager.addSegment((ImmutableSegment) indexSegment);
}
InstanceDataManager instanceDataManager = mock(InstanceDataManager.class);
- when(instanceDataManager.getTableDataManager(RAW_TABLE_NAME)).thenReturn(tableDataManager);
+ when(instanceDataManager.getTableDataManager(OFFLINE_TABLE_NAME)).thenReturn(tableDataManager);
Review Comment:
This test was not well implemented because we should use table name with type on the server side. Fixed the test
--
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
[GitHub] [pinot] codecov-commenter commented on pull request #9920: Clean up TableDataManagerConfig to be a thin wrapper over instance and table config
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9920:
URL: https://github.com/apache/pinot/pull/9920#issuecomment-1340126801
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9920?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#9920](https://codecov.io/gh/apache/pinot/pull/9920?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e66fc1e) into [master](https://codecov.io/gh/apache/pinot/commit/e6a9881d7d1b397934983756ca4f14f3419d6824?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6a9881) will **decrease** coverage by `43.94%`.
> The diff coverage is `4.34%`.
```diff
@@ Coverage Diff @@
## master #9920 +/- ##
=============================================
- Coverage 68.97% 25.03% -43.95%
+ Complexity 5058 44 -5014
=============================================
Files 1982 1970 -12
Lines 106433 106062 -371
Branches 16127 16089 -38
=============================================
- Hits 73409 26549 -46860
- Misses 27877 76816 +48939
+ Partials 5147 2697 -2450
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `25.03% <4.34%> (?)` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9920?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/core/data/manager/BaseTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvQmFzZVRhYmxlRGF0YU1hbmFnZXIuamF2YQ==) | `57.10% <0.00%> (-17.48%)` | :arrow_down: |
| [...data/manager/offline/TableDataManagerProvider.java](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9UYWJsZURhdGFNYW5hZ2VyUHJvdmlkZXIuamF2YQ==) | `66.66% <0.00%> (ø)` | |
| [...ent/local/data/manager/TableDataManagerConfig.java](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9kYXRhL21hbmFnZXIvVGFibGVEYXRhTWFuYWdlckNvbmZpZy5qYXZh) | `0.00% <0.00%> (-26.32%)` | :arrow_down: |
| [...server/starter/helix/HelixInstanceDataManager.java](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeEluc3RhbmNlRGF0YU1hbmFnZXIuamF2YQ==) | `64.28% <100.00%> (-6.55%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/user/RoleType.java](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3VzZXIvUm9sZVR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1501 more](https://codecov.io/gh/apache/pinot/pull/9920/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9920: Clean up TableDataManagerConfig to be a thin wrapper over instance and table config
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9920:
URL: https://github.com/apache/pinot/pull/9920#discussion_r1041592186
##########
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:
Yes. This PR can replace #9918, and has more cleanups
--
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