You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by su...@apache.org on 2018/12/07 03:57:17 UTC

[incubator-pinot] 01/01: [PINOT-7461] Fix segment deletion when folder under Deleted_Segments location does not exist

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

sunithabeeram pushed a commit to branch segmentDeletionFix
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git

commit a2442fe092c39b011e6249fe05ea003bca381bf3
Author: Sunitha Beeram <sb...@linkedin.com>
AuthorDate: Thu Dec 6 19:56:54 2018 -0800

    [PINOT-7461] Fix segment deletion when folder under Deleted_Segments location does not exist
---
 .../linkedin/pinot/filesystem/LocalPinotFS.java    |  3 ++
 .../pinot/filesystem/LocalPinotFSTest.java         | 34 ++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/pinot-filesystem/src/main/java/com/linkedin/pinot/filesystem/LocalPinotFS.java b/pinot-filesystem/src/main/java/com/linkedin/pinot/filesystem/LocalPinotFS.java
index 1329a3d..7ff2c19 100644
--- a/pinot-filesystem/src/main/java/com/linkedin/pinot/filesystem/LocalPinotFS.java
+++ b/pinot-filesystem/src/main/java/com/linkedin/pinot/filesystem/LocalPinotFS.java
@@ -80,6 +80,9 @@ public class LocalPinotFS extends PinotFS {
         // dst file exists, returning
         return false;
       }
+    } else {
+      // ensure the dst path exists
+      FileUtils.forceMkdir(dstFile.getParentFile());
     }
 
     Files.move(srcFile.toPath(), dstFile.toPath());
diff --git a/pinot-filesystem/src/test/java/com/linkedin/pinot/filesystem/LocalPinotFSTest.java b/pinot-filesystem/src/test/java/com/linkedin/pinot/filesystem/LocalPinotFSTest.java
index 0d2f7f3..81a4800 100644
--- a/pinot-filesystem/src/test/java/com/linkedin/pinot/filesystem/LocalPinotFSTest.java
+++ b/pinot-filesystem/src/test/java/com/linkedin/pinot/filesystem/LocalPinotFSTest.java
@@ -31,6 +31,7 @@ public class LocalPinotFSTest {
   private File testFile;
   private File _absoluteTmpDirPath;
   private File _newTmpDir;
+  private File _nonExistentTmpFolder;
 
   @BeforeClass
   public void setUp() {
@@ -48,8 +49,11 @@ public class LocalPinotFSTest {
     FileUtils.deleteQuietly(_newTmpDir);
     Assert.assertTrue(_newTmpDir.mkdir(), "Could not make directory " + _newTmpDir.getPath());
 
+    _nonExistentTmpFolder = new File(System.getProperty("java.io.tmpdir"), LocalPinotFSTest.class.getSimpleName() + "nonExistentParent/nonExistent");
+
     _absoluteTmpDirPath.deleteOnExit();
     _newTmpDir.deleteOnExit();
+    _nonExistentTmpFolder.deleteOnExit();
   }
 
   @AfterClass
@@ -117,12 +121,42 @@ public class LocalPinotFSTest {
 
     // Check that method deletes dst directory during move and is successful by overwriting dir
     Assert.assertTrue(_newTmpDir.exists());
+    // create a file in the dst folder
+    File dstFile = new File(_newTmpDir.getPath() + "/newFile" );
+    dstFile.createNewFile();
 
     // Expected that a move without overwrite will not succeed
     Assert.assertFalse(localPinotFS.move(_absoluteTmpDirPath.toURI(), _newTmpDir.toURI(), false));
 
+    int files = _absoluteTmpDirPath.listFiles().length;
     Assert.assertTrue(localPinotFS.move(_absoluteTmpDirPath.toURI(), _newTmpDir.toURI(), true));
     Assert.assertEquals(_absoluteTmpDirPath.length(), 0);
+    Assert.assertEquals(_newTmpDir.listFiles().length, files);
+    Assert.assertFalse(dstFile.exists());
+
+    // Check that a moving a file a non-existent destination folder will work
+    FileUtils.deleteQuietly(_nonExistentTmpFolder);
+    Assert.assertFalse(_nonExistentTmpFolder.exists());
+    File srcFile = new File(_absoluteTmpDirPath, "srcFile");
+    localPinotFS.mkdir(_absoluteTmpDirPath.toURI());
+    Assert.assertTrue(srcFile.createNewFile());
+    dstFile = new File(_nonExistentTmpFolder.getPath() + "/newFile" );
+    Assert.assertFalse(dstFile.exists());
+    Assert.assertTrue(localPinotFS.move(srcFile.toURI(), dstFile.toURI(), true)); // overwrite flag has no impact
+    Assert.assertFalse(srcFile.exists());
+    Assert.assertTrue(dstFile.exists());
+
+    //Check that moving a folder to a non-existent destination folder works
+    FileUtils.deleteQuietly(_nonExistentTmpFolder);
+    Assert.assertFalse(_nonExistentTmpFolder.exists());
+    srcFile = new File(_absoluteTmpDirPath, "srcFile");
+    localPinotFS.mkdir(_absoluteTmpDirPath.toURI());
+    Assert.assertTrue(srcFile.createNewFile());
+    dstFile = new File(_nonExistentTmpFolder.getPath() + "/srcFile" );
+    Assert.assertFalse(dstFile.exists());
+    Assert.assertTrue(localPinotFS.move(_absoluteTmpDirPath.toURI(), _nonExistentTmpFolder.toURI(),
+        true)); // overwrite flag has no impact
+    Assert.assertTrue(dstFile.exists());
 
     localPinotFS.delete(secondTestFileUri, true);
     // Check deletion from final location worked


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