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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
