You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ma...@apache.org on 2024/11/12 15:36:42 UTC
(pinot) branch master updated: Cleanup UpsertCompaction validations during task-generation (#14424)
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 <ti...@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-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org