You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ne...@apache.org on 2022/02/04 20:29:54 UTC

[pinot] branch master updated: disallow duplicate segment name in tar file (#8119)

This is an automated email from the ASF dual-hosted git repository.

nehapawar 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 cac657a  disallow duplicate segment name in tar file (#8119)
cac657a is described below

commit cac657adec6a9220c0afacf711962c551ab08caf
Author: Rong Rong <ro...@apache.org>
AuthorDate: Fri Feb 4 12:29:36 2022 -0800

    disallow duplicate segment name in tar file (#8119)
    
    Co-authored-by: Rong Rong <ro...@startree.ai>
---
 .../segmentwriter/filebased/FileBasedSegmentWriter.java   | 15 ++++++++++++---
 .../filebased/FileBasedSegmentWriterTest.java             | 11 ++++++-----
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/pinot-plugins/pinot-segment-writer/pinot-segment-writer-file-based/src/main/java/org/apache/pinot/plugin/segmentwriter/filebased/FileBasedSegmentWriter.java b/pinot-plugins/pinot-segment-writer/pinot-segment-writer-file-based/src/main/java/org/apache/pinot/plugin/segmentwriter/filebased/FileBasedSegmentWriter.java
index f19f0d7..966d8ba 100644
--- a/pinot-plugins/pinot-segment-writer/pinot-segment-writer-file-based/src/main/java/org/apache/pinot/plugin/segmentwriter/filebased/FileBasedSegmentWriter.java
+++ b/pinot-plugins/pinot-segment-writer/pinot-segment-writer-file-based/src/main/java/org/apache/pinot/plugin/segmentwriter/filebased/FileBasedSegmentWriter.java
@@ -194,9 +194,18 @@ public class FileBasedSegmentWriter implements SegmentWriter {
 
       // Tar segment
       File segmentTarFile = new File(_outputDirURI, segmentName + Constants.TAR_GZ_FILE_EXT);
-      if (!_batchConfig.isOverwriteOutput() && segmentTarFile.exists()) {
-        segmentTarFile = new File(_outputDirURI,
-            String.format("%s_%d%s", segmentName, System.currentTimeMillis(), Constants.TAR_GZ_FILE_EXT));
+      if (segmentTarFile.exists()) {
+        if (!_batchConfig.isOverwriteOutput()) {
+          throw new IllegalArgumentException(String.format("Duplicate segment name generated '%s' in '%s', please "
+              + "adjust segment name generator config to avoid duplicates, or allow batch config overwrite",
+              segmentName, _outputDirURI));
+        } else {
+          LOGGER.warn(String.format("Duplicate segment name detected '%s' in file '%s', deleting old segment",
+              segmentName, segmentDir));
+          if (segmentTarFile.delete()) {
+            LOGGER.warn(String.format("Segment file deleted: '%s/%s'", _outputDirURI, segmentName));
+          }
+        }
       }
       TarGzCompressionUtils.createTarGzFile(new File(segmentDir, segmentName), segmentTarFile);
       LOGGER.info("Created segment tar: {} for segment: {} of table: {}", segmentTarFile.getAbsolutePath(), segmentName,
diff --git a/pinot-plugins/pinot-segment-writer/pinot-segment-writer-file-based/src/test/java/org/apache/pinot/plugin/segmentwriter/filebased/FileBasedSegmentWriterTest.java b/pinot-plugins/pinot-segment-writer/pinot-segment-writer-file-based/src/test/java/org/apache/pinot/plugin/segmentwriter/filebased/FileBasedSegmentWriterTest.java
index 12f02af..7383378 100644
--- a/pinot-plugins/pinot-segment-writer/pinot-segment-writer-file-based/src/test/java/org/apache/pinot/plugin/segmentwriter/filebased/FileBasedSegmentWriterTest.java
+++ b/pinot-plugins/pinot-segment-writer/pinot-segment-writer-file-based/src/test/java/org/apache/pinot/plugin/segmentwriter/filebased/FileBasedSegmentWriterTest.java
@@ -381,11 +381,12 @@ public class FileBasedSegmentWriterTest {
     segmentWriter.collect(getGenericRow("record7", 1616238000000L));
     segmentWriter.collect(getGenericRow("record8", 1616238000000L));
     segmentWriter.collect(getGenericRow("record9", 1616238000000L));
-    segmentWriter.flush();
-
-    // verify tar was not overwritten
-    segmentTars = _outputDir.listFiles();
-    Assert.assertEquals(segmentTars.length, 2);
+    try {
+      segmentWriter.flush();
+      Assert.fail();
+    } catch (RuntimeException e) {
+      // expected.
+    }
 
     segmentWriter.close();
     FileUtils.deleteQuietly(_outputDir);

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org