This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang 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 0ac943c4304 Respect explicit append.uuid.to.segment.name in
SegmentGenerationAndPushTask (#19820)
0ac943c4304 is described below
commit 0ac943c4304c128b8f8f9b98d38e9416344b0641
Author: Akshay Upadhyay <[email protected]>
AuthorDate: Tue Oct 13 03:08:30 2026 +0530
Respect explicit append.uuid.to.segment.name in
SegmentGenerationAndPushTask (#19820)
---
.../SegmentGenerationAndPushTaskExecutor.java | 8 +-
.../SegmentGenerationAndPushTaskGenerator.java | 11 +-
.../SegmentGenerationAndPushTaskExecutorTest.java | 37 +++++
.../SegmentGenerationAndPushTaskGeneratorTest.java | 151 +++++++++++++++++++++
4 files changed, 203 insertions(+), 4 deletions(-)
diff --git
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskExecutor.java
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskExecutor.java
index 5d9825aeb3d..c4dcd5f76b1 100644
---
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskExecutor.java
+++
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskExecutor.java
@@ -315,8 +315,12 @@ public class SegmentGenerationAndPushTaskExecutor extends
BaseTaskExecutor {
segmentNameGeneratorSpec.setType(taskConfigs.get(BatchConfigProperties.SEGMENT_NAME_GENERATOR_TYPE));
segmentNameGeneratorSpec.setConfigs(IngestionConfigUtils.getConfigMapWithPrefix(taskConfigs,
BatchConfigProperties.SEGMENT_NAME_GENERATOR_PROP_PREFIX));
-
segmentNameGeneratorSpec.addConfig(SegmentGenerationTaskRunner.APPEND_UUID_TO_SEGMENT_NAME,
-
taskConfigs.getOrDefault(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
Boolean.toString(false)));
+ // An explicit segmentNameGenerator.configs.append.uuid.to.segment.name
takes precedence over the top-level key
+ if (!segmentNameGeneratorSpec.getConfigs()
+
.containsKey(SegmentGenerationTaskRunner.APPEND_UUID_TO_SEGMENT_NAME)) {
+
segmentNameGeneratorSpec.addConfig(SegmentGenerationTaskRunner.APPEND_UUID_TO_SEGMENT_NAME,
+
taskConfigs.getOrDefault(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
Boolean.toString(false)));
+ }
taskSpec.setSegmentNameGeneratorSpec(segmentNameGeneratorSpec);
taskSpec.setCustomProperty(BatchConfigProperties.INPUT_DATA_FILE_URI_KEY,
inputFileURI.toString());
diff --git
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskGenerator.java
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskGenerator.java
index b3d9b1f2050..6f1cf20d50a 100644
---
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskGenerator.java
+++
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskGenerator.java
@@ -286,8 +286,15 @@ public class SegmentGenerationAndPushTaskGenerator extends
BaseTaskGenerator {
// overridden, corresponding files become un-ingested, the generator will
generate new tasks, which generates
// segments with same names, and override existing segments again... It
becomes an endless loop
// We add uuid to segment name to avoid segment collision across multiple
rounds of task generation to solve the
- // problem.
-
singleFileGenerationTaskConfig.put(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
Boolean.toString(true));
+ // problem. Default to true, but respect an explicit value configured in
the batch config, either under
+ // segmentNameGenerator.configs (takes precedence) or top-level. Note that
explicitly setting it to false makes
+ // segment names repeat across rounds unless the segment name generator
produces unique names per input.
+ String appendUuidConfigKey =
BatchConfigProperties.SEGMENT_NAME_GENERATOR_PROP_PREFIX + "."
+ + BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME;
+
singleFileGenerationTaskConfig.put(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
+ singleFileGenerationTaskConfig.getOrDefault(appendUuidConfigKey,
+
singleFileGenerationTaskConfig.getOrDefault(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
+ Boolean.toString(true))));
if ((outputDirURI == null) || (pushMode == null)) {
singleFileGenerationTaskConfig.put(BatchConfigProperties.PUSH_MODE,
DEFAULT_SEGMENT_PUSH_TYPE.toString());
} else {
diff --git
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskExecutorTest.java
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskExecutorTest.java
index b497fc54c85..5a097b07157 100644
---
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskExecutorTest.java
+++
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskExecutorTest.java
@@ -86,4 +86,41 @@ public class SegmentGenerationAndPushTaskExecutorTest {
Map.of("prop.seg.1", "valseg1", "propseg2", "valseg2",
SegmentGenerationTaskRunner.APPEND_UUID_TO_SEGMENT_NAME,
"true"));
}
+
+ @Test
+ public void testGenerateTaskSpecAppendUuidPrecedence()
+ throws Exception {
+ // Explicit prefixed config wins over the top-level key
+ assertAppendUuid(Map.of(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
"true",
+ BatchConfigProperties.SEGMENT_NAME_GENERATOR_PROP_PREFIX + "."
+ + SegmentGenerationTaskRunner.APPEND_UUID_TO_SEGMENT_NAME,
"false"), "false");
+ // Top-level key is still honored when the prefixed config is absent
+ assertAppendUuid(Map.of(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
"true"), "true");
+ assertAppendUuid(Map.of(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
"false"), "false");
+ // Unchanged default when neither is set
+ assertAppendUuid(Map.of(), "false");
+ }
+
+ private void assertAppendUuid(Map<String, String> appendUuidConfigs, String
expected)
+ throws Exception {
+ ClassLoader classLoader = getClass().getClassLoader();
+ URL resourcesLoc = classLoader.getResource(".");
+ assertNotNull(resourcesLoc);
+ URL tableConfigUrl = classLoader.getResource("dummyTable.json");
+ assertNotNull(tableConfigUrl);
+ SegmentGenerationAndPushTaskExecutor executor = new
SegmentGenerationAndPushTaskExecutor();
+ FieldUtils.writeField(executor, "_eventObserver", new
DefaultMinionEventObserver(), true);
+ Map<String, String> configMap = new HashMap<>(appendUuidConfigs);
+ configMap.put(BatchConfigProperties.INPUT_DATA_FILE_URI_KEY, resourcesLoc
+ "dummyTable.json");
+ configMap.put(BatchConfigProperties.RECORD_READER_CLASS, "AReaderClass");
+ configMap.put(BatchConfigProperties.RECORD_READER_CONFIG_CLASS,
"AReaderConfigClass");
+ configMap.put(BatchConfigProperties.SCHEMA, new
Schema.SchemaBuilder().build().toSingleLineJsonString());
+ configMap.put(BatchConfigProperties.TABLE_CONFIGS,
+ FileUtils.readFileToString(new File(tableConfigUrl.getFile()),
StandardCharsets.UTF_8));
+ configMap.put(BatchConfigProperties.SEQUENCE_ID, "42");
+ configMap.put(BatchConfigProperties.SEGMENT_NAME_GENERATOR_TYPE,
"inputFile");
+ SegmentGenerationTaskSpec spec = executor.generateTaskSpec(configMap,
Paths.get(resourcesLoc.toURI()).toFile());
+ assertEquals(spec.getSegmentNameGeneratorSpec().getConfigs()
+ .get(SegmentGenerationTaskRunner.APPEND_UUID_TO_SEGMENT_NAME),
expected);
+ }
}
diff --git
a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskGeneratorTest.java
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskGeneratorTest.java
new file mode 100644
index 00000000000..156b0f62c7c
--- /dev/null
+++
b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskGeneratorTest.java
@@ -0,0 +1,151 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.plugin.minion.tasks.segmentgenerationandpush;
+
+import java.io.File;
+import java.net.URI;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.commons.lang3.reflect.MethodUtils;
+import org.apache.pinot.controller.helix.core.minion.ClusterInfoAccessor;
+import org.apache.pinot.minion.event.DefaultMinionEventObserver;
+import
org.apache.pinot.plugin.ingestion.batch.common.SegmentGenerationTaskRunner;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.ingestion.batch.BatchConfigProperties;
+import org.apache.pinot.spi.ingestion.batch.spec.SegmentGenerationTaskSpec;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+
+
+/// Tests for [SegmentGenerationAndPushTaskGenerator]
+public class SegmentGenerationAndPushTaskGeneratorTest {
+
+ private static final String TABLE_NAME = "testTable_OFFLINE";
+ private static final URI INPUT_FILE_URI =
URI.create("file:///tmp/input/data.csv");
+ private static final String APPEND_UUID_CONFIG_KEY =
+ BatchConfigProperties.SEGMENT_NAME_GENERATOR_PROP_PREFIX + "."
+ + BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME;
+
+ private static final String APPEND_UUID_TO_SEGMENT_NAME_KEY =
SegmentGenerationTaskRunner.APPEND_UUID_TO_SEGMENT_NAME;
+
+ private SegmentGenerationAndPushTaskGenerator _generator;
+
+ @BeforeMethod
+ public void setUp() {
+ ClusterInfoAccessor clusterInfoAccessor = mock(ClusterInfoAccessor.class);
+
when(clusterInfoAccessor.getVipUrlForLeadController(TABLE_NAME)).thenReturn("http://localhost:9000";);
+ _generator = new SegmentGenerationAndPushTaskGenerator();
+ _generator.init(clusterInfoAccessor);
+ }
+
+ @Test
+ public void testAppendUuidDefaultsToTrueWhenUnset()
+ throws Exception {
+ Map<String, String> taskConfig =
invokeGetSingleFileGenerationTaskConfig(batchConfigMap(Map.of()));
+
assertEquals(taskConfig.get(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME),
"true");
+ }
+
+ @Test
+ public void testAppendUuidRespectsExplicitTopLevelFalse()
+ throws Exception {
+ Map<String, String> taskConfig = invokeGetSingleFileGenerationTaskConfig(
+
batchConfigMap(Map.of(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
"false")));
+
assertEquals(taskConfig.get(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME),
"false");
+ }
+
+ @Test
+ public void testAppendUuidRespectsExplicitPrefixedFalse()
+ throws Exception {
+ Map<String, String> taskConfig =
+
invokeGetSingleFileGenerationTaskConfig(batchConfigMap(Map.of(APPEND_UUID_CONFIG_KEY,
"false")));
+ assertEquals(taskConfig.get(APPEND_UUID_CONFIG_KEY), "false");
+ // Mirrored to the top-level key so that executors which only read the
top-level key behave the same
+
assertEquals(taskConfig.get(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME),
"false");
+ }
+
+ @Test
+ public void testAppendUuidPrefixedTakesPrecedenceOverTopLevel()
+ throws Exception {
+ Map<String, String> taskConfig = invokeGetSingleFileGenerationTaskConfig(
+
batchConfigMap(Map.of(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
"true", APPEND_UUID_CONFIG_KEY,
+ "false")));
+
assertEquals(taskConfig.get(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME),
"false");
+ assertEquals(taskConfig.get(APPEND_UUID_CONFIG_KEY), "false");
+ }
+
+ @Test
+ public void testGeneratedTaskConfigIsHonoredByExecutor()
+ throws Exception {
+ // Table config -> generator -> executor: the spec consumed by the segment
generation runner must carry the
+ // configured value
+ assertAppendUuidInExecutorSpec(Map.of(APPEND_UUID_CONFIG_KEY, "false"),
"false");
+
assertAppendUuidInExecutorSpec(Map.of(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
"false"), "false");
+
assertAppendUuidInExecutorSpec(Map.of(BatchConfigProperties.APPEND_UUID_TO_SEGMENT_NAME,
"true",
+ APPEND_UUID_CONFIG_KEY, "false"), "false");
+ assertAppendUuidInExecutorSpec(Map.of(), "true");
+ }
+
+ private static Map<String, String> batchConfigMap(Map<String, String>
extraConfigs) {
+ Map<String, String> batchConfigMap = new HashMap<>(extraConfigs);
+ batchConfigMap.put(BatchConfigProperties.INPUT_DIR_URI,
"file:///tmp/input");
+ batchConfigMap.put(BatchConfigProperties.INPUT_FORMAT, "csv");
+ batchConfigMap.put(BatchConfigProperties.SEGMENT_NAME_GENERATOR_TYPE,
"inputFile");
+ return batchConfigMap;
+ }
+
+ private void assertAppendUuidInExecutorSpec(Map<String, String>
extraBatchConfigs, String expected)
+ throws Exception {
+ // Add the fields the executor normally receives from the controller
+ Map<String, String> taskConfig =
+ new
HashMap<>(invokeGetSingleFileGenerationTaskConfig(batchConfigMap(extraBatchConfigs)));
+ ClassLoader classLoader = getClass().getClassLoader();
+ URL resourcesLoc = classLoader.getResource(".");
+ assertNotNull(resourcesLoc);
+ URL tableConfigUrl = classLoader.getResource("dummyTable.json");
+ assertNotNull(tableConfigUrl);
+ taskConfig.put(BatchConfigProperties.INPUT_DATA_FILE_URI_KEY, resourcesLoc
+ "dummyTable.json");
+ taskConfig.put(BatchConfigProperties.RECORD_READER_CLASS, "AReaderClass");
+ taskConfig.put(BatchConfigProperties.RECORD_READER_CONFIG_CLASS,
"AReaderConfigClass");
+ taskConfig.put(BatchConfigProperties.SCHEMA, new
Schema.SchemaBuilder().build().toSingleLineJsonString());
+ taskConfig.put(BatchConfigProperties.TABLE_CONFIGS,
+ FileUtils.readFileToString(new File(tableConfigUrl.getFile()),
StandardCharsets.UTF_8));
+
+ SegmentGenerationAndPushTaskExecutor executor = new
SegmentGenerationAndPushTaskExecutor();
+ FieldUtils.writeField(executor, "_eventObserver", new
DefaultMinionEventObserver(), true);
+ SegmentGenerationTaskSpec spec = executor.generateTaskSpec(taskConfig,
Paths.get(resourcesLoc.toURI()).toFile());
+
assertEquals(spec.getSegmentNameGeneratorSpec().getConfigs().get(APPEND_UUID_TO_SEGMENT_NAME_KEY),
expected);
+ }
+
+ @SuppressWarnings("unchecked")
+ private Map<String, String>
invokeGetSingleFileGenerationTaskConfig(Map<String, String> batchConfigMap)
+ throws Exception {
+ return (Map<String, String>) MethodUtils.invokeMethod(_generator, true,
"getSingleFileGenerationTaskConfig",
+ TABLE_NAME, 0, batchConfigMap, INPUT_FILE_URI, null);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
