This is an automated email from the ASF dual-hosted git repository.
manishswaminathan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 0180a11bba Cleanup UpsertCompaction validations during task-generation
(#14424)
0180a11bba is described below
commit 0180a11bba4c98e5af6131807fddf263669f92b1
Author: Pratik Tibrewal <tibrewalprati...@gmail.com>
AuthorDate: Tue Nov 12 21:06:35 2024 +0530
Cleanup UpsertCompaction validations during task-generation (#14424)
---
.../UpsertCompactionTaskGenerator.java | 36 ----------------------
.../UpsertCompactionTaskGeneratorTest.java | 16 ----------
2 files changed, 52 deletions(-)
diff --git
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java
index ae74bc8ca9..d289ccf01c 100644
---
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java
+++
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java
@@ -90,10 +90,6 @@ public class UpsertCompactionTaskGenerator extends
BaseTaskGenerator {
String taskType = MinionConstants.UpsertCompactionTask.TASK_TYPE;
List<PinotTaskConfig> pinotTaskConfigs = new ArrayList<>();
for (TableConfig tableConfig : tableConfigs) {
- if (!validate(tableConfig)) {
- LOGGER.warn("Validation failed for table {}. Skipping..",
tableConfig.getTableName());
- continue;
- }
String tableNameWithType = tableConfig.getTableName();
LOGGER.info("Start generating task configs for table: {}",
tableNameWithType);
@@ -150,21 +146,6 @@ public class UpsertCompactionTaskGenerator extends
BaseTaskGenerator {
taskConfigs.getOrDefault(UpsertCompactionTask.NUM_SEGMENTS_BATCH_PER_SERVER_REQUEST,
String.valueOf(DEFAULT_NUM_SEGMENTS_BATCH_PER_SERVER_REQUEST)));
- // Validate that the snapshot is enabled if validDocIdsType is
validDocIdsSnapshot
- if (validDocIdsType == ValidDocIdsType.SNAPSHOT) {
- UpsertConfig upsertConfig = tableConfig.getUpsertConfig();
- Preconditions.checkNotNull(upsertConfig, "UpsertConfig must be
provided for UpsertCompactionTask");
- Preconditions.checkState(upsertConfig.isEnableSnapshot(),
String.format(
- "'enableSnapshot' from UpsertConfig must be enabled for
UpsertCompactionTask with validDocIdsType = %s",
- validDocIdsType));
- } else if (validDocIdsType == ValidDocIdsType.IN_MEMORY_WITH_DELETE) {
- UpsertConfig upsertConfig = tableConfig.getUpsertConfig();
- Preconditions.checkNotNull(upsertConfig, "UpsertConfig must be
provided for UpsertCompactionTask");
- Preconditions.checkNotNull(upsertConfig.getDeleteRecordColumn(),
- String.format("deleteRecordColumn must be provided for " +
"UpsertCompactionTask with validDocIdsType = %s",
- validDocIdsType));
- }
-
Map<String, List<ValidDocIdsMetadataInfo>> validDocIdsMetadataList =
serverSegmentMetadataReader.getSegmentToValidDocIdsMetadataFromServer(tableNameWithType,
serverToSegments,
serverToEndpoints, null, 60_000, validDocIdsType.toString(),
numSegmentsBatchPerServerRequest);
@@ -288,23 +269,6 @@ public class UpsertCompactionTaskGenerator extends
BaseTaskGenerator {
return maxTasks;
}
- @VisibleForTesting
- static boolean validate(TableConfig tableConfig) {
- String taskType = MinionConstants.UpsertCompactionTask.TASK_TYPE;
- String tableNameWithType = tableConfig.getTableName();
- if (tableConfig.getTableType() == TableType.OFFLINE) {
- LOGGER.warn("Skip generation task: {} for table: {}, offline table is
not supported", taskType,
- tableNameWithType);
- return false;
- }
- if (!tableConfig.isUpsertEnabled()) {
- LOGGER.warn("Skip generation task: {} for table: {}, table without
upsert enabled is not supported", taskType,
- tableNameWithType);
- return false;
- }
- return true;
- }
-
@Override
public void validateTaskConfigs(TableConfig tableConfig, Map<String, String>
taskConfigs) {
// check table is realtime
diff --git
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGeneratorTest.java
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGeneratorTest.java
index 604c1aa476..1204c5ae5f 100644
---
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGeneratorTest.java
+++
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGeneratorTest.java
@@ -50,7 +50,6 @@ import org.testng.collections.Lists;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
@@ -98,21 +97,6 @@ public class UpsertCompactionTaskGeneratorTest {
_completedSegmentsMap.put(_completedSegment2.getSegmentName(),
_completedSegment2);
}
- @Test
- public void testValidate() {
- TableConfig tableConfig =
- new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).setTimeColumnName(TIME_COLUMN_NAME)
- .build();
- assertFalse(UpsertCompactionTaskGenerator.validate(tableConfig));
-
- TableConfigBuilder tableConfigBuilder =
- new
TableConfigBuilder(TableType.REALTIME).setTableName(RAW_TABLE_NAME).setTimeColumnName(TIME_COLUMN_NAME);
-
assertFalse(UpsertCompactionTaskGenerator.validate(tableConfigBuilder.build()));
-
- tableConfigBuilder = tableConfigBuilder.setUpsertConfig(new
UpsertConfig(UpsertConfig.Mode.FULL));
-
assertTrue(UpsertCompactionTaskGenerator.validate(tableConfigBuilder.build()));
- }
-
@Test
public void testGenerateTasksValidatesTableConfigs() {
UpsertCompactionTaskGenerator taskGenerator = new
UpsertCompactionTaskGenerator();
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
