From f46621fee439834828a74d9cc6b0ccb8f4fe20e7 Mon Sep 17 00:00:00 2001
From: brinn <brinn>
Date: Sun, 2 Sep 2007 17:09:49 +0000
Subject: [PATCH] DMV-8: Recovery after partial deletion can damage data This
is kind of a hack to resolve the partial deletion situation that introduces
another status file.
SVN: 1568
---
.../cisd/datamover/MonitorStarter.java | 26 ++--
.../cisd/datamover/RemotePathMover.java | 117 ++++++++++++++----
...opyFinishedMarker.java => MarkerFile.java} | 32 +++--
.../ch/systemsx/cisd/datamover/MainTest.java | 33 ++++-
.../testhelper/FileStructEngine.java | 25 ++--
5 files changed, 176 insertions(+), 57 deletions(-)
rename datamover/source/java/ch/systemsx/cisd/datamover/helper/{CopyFinishedMarker.java => MarkerFile.java} (53%)
diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/MonitorStarter.java b/datamover/source/java/ch/systemsx/cisd/datamover/MonitorStarter.java
index 7a74d203f11..50d3c9f0f8c 100644
--- a/datamover/source/java/ch/systemsx/cisd/datamover/MonitorStarter.java
+++ b/datamover/source/java/ch/systemsx/cisd/datamover/MonitorStarter.java
@@ -22,7 +22,7 @@ import java.util.Timer;
import ch.systemsx.cisd.common.utilities.DirectoryScanningTimerTask;
import ch.systemsx.cisd.common.utilities.ITerminable;
import ch.systemsx.cisd.common.utilities.DirectoryScanningTimerTask.IPathHandler;
-import ch.systemsx.cisd.datamover.helper.CopyFinishedMarker;
+import ch.systemsx.cisd.datamover.helper.MarkerFile;
import ch.systemsx.cisd.datamover.helper.FileSystemHelper;
import ch.systemsx.cisd.datamover.intf.IFileSysOperationsFactory;
import ch.systemsx.cisd.datamover.intf.IPathCopier;
@@ -153,6 +153,10 @@ public class MonitorStarter
for (File file : files)
{
+ if (MarkerFile.isDeletionInProgressMarker(file))
+ {
+ continue;
+ }
recoverIncomingAfterShutdown(file, incomingStore, incomingReadOperations, copyCompleteDir, prefixTemplate);
}
}
@@ -160,10 +164,10 @@ public class MonitorStarter
private static void recoverIncomingAfterShutdown(File unfinishedFile, FileStore incomingStore,
IReadPathOperations incomingReadOperations, File copyCompleteDir, String prefixTemplate)
{
- if (CopyFinishedMarker.isMarker(unfinishedFile))
+ if (MarkerFile.isCopyFinishedMarker(unfinishedFile))
{
final File markerFile = unfinishedFile;
- final File localCopy = CopyFinishedMarker.extractOriginal(markerFile);
+ final File localCopy = MarkerFile.extractOriginalFromCopyFinishedMarker(markerFile);
if (localCopy.exists())
{
// copy and marker exist - do nothing, recovery will be done for copied resource
@@ -176,7 +180,7 @@ public class MonitorStarter
// handle local copy
{
final File localCopy = unfinishedFile;
- final File markerFile = CopyFinishedMarker.extractMarker(localCopy);
+ final File markerFile = MarkerFile.createCopyFinishedMarker(localCopy);
if (markerFile.exists())
{
// copy and marker exist - copy finished, but copied resource not moved
@@ -207,9 +211,13 @@ public class MonitorStarter
return; // directory is empty, no recovery is needed
}
- for (int i = 0; i < files.length; i++)
+ for (File file : files)
{
- localProcessor.handle(files[i]);
+ if (MarkerFile.isDeletionInProgressMarker(file))
+ {
+ continue;
+ }
+ localProcessor.handle(file);
}
}
@@ -252,9 +260,9 @@ public class MonitorStarter
}
FileSystemHelper.createDestinationPath(source, null, copyInProgressDir, parameters.getPrefixForIncoming());
final File copiedFile = new File(copyInProgressDir, source.getName());
- assert copiedFile.exists();
- final File markerFile = CopyFinishedMarker.extractMarker(copiedFile);
- assert markerFile.exists();
+ assert copiedFile.exists() : copiedFile.getAbsolutePath();
+ final File markerFile = MarkerFile.createCopyFinishedMarker(copiedFile);
+ assert markerFile.exists() : markerFile.getAbsolutePath();
// 2. Move to final directory, delete marker
final File finalFile =
diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/RemotePathMover.java b/datamover/source/java/ch/systemsx/cisd/datamover/RemotePathMover.java
index 62752cb969f..2770ffde07f 100644
--- a/datamover/source/java/ch/systemsx/cisd/datamover/RemotePathMover.java
+++ b/datamover/source/java/ch/systemsx/cisd/datamover/RemotePathMover.java
@@ -27,7 +27,7 @@ import ch.systemsx.cisd.common.logging.LogCategory;
import ch.systemsx.cisd.common.logging.LogFactory;
import ch.systemsx.cisd.common.utilities.DirectoryScanningTimerTask;
import ch.systemsx.cisd.common.utilities.FileUtilities;
-import ch.systemsx.cisd.datamover.helper.CopyFinishedMarker;
+import ch.systemsx.cisd.datamover.helper.MarkerFile;
import ch.systemsx.cisd.datamover.intf.IPathCopier;
import ch.systemsx.cisd.datamover.intf.IPathRemover;
import ch.systemsx.cisd.datamover.intf.ITimingParameters;
@@ -122,6 +122,18 @@ public final class RemotePathMover implements DirectoryScanningTimerTask.IPathHa
public boolean handle(File path)
{
+ if (isDeletionInProgress(path))
+ {
+ // That is a recovery situation: we have been interrupted removing the path and now finish the job.
+ if (operationLog.isInfoEnabled())
+ {
+ operationLog.info(String.format(
+ "Detected recovery situation: '%s' has been interrupted in deletion phase, finishing up.", path
+ .getAbsolutePath()));
+ }
+ remove(path);
+ return markAsFinished(path);
+ }
int tryCount = 0;
do
{
@@ -149,15 +161,7 @@ public final class RemotePathMover implements DirectoryScanningTimerTask.IPathHa
operationLog.info(String.format(FINISH_COPYING_PATH_TEMPLATE, path.getPath(), destinationDirectory
.getPath(), (endTime - startTime) / 1000.0));
}
- final Status removalStatus = remover.remove(path);
- if (Status.OK.equals(removalStatus) == false)
- {
- // We don't retry this, because the path is local and removal really shouldn't fail.
- notificationLog.error(String.format(REMOVING_LOCAL_PATH_FAILED_TEMPLATE, path, removalStatus));
- } else if (operationLog.isInfoEnabled())
- {
- operationLog.info(String.format(REMOVED_PATH_TEMPLATE, path.getPath()));
- }
+ remove(path);
// Note: we return true even if removal of the directory failed. There is no point in retrying the
// operation.
return markAsFinished(path);
@@ -189,62 +193,121 @@ public final class RemotePathMover implements DirectoryScanningTimerTask.IPathHa
return false;
}
+ private void remove(File path)
+ {
+ final File removalInProgressMarkerFile = tryMarkAsDeletionInProgress(path);
+ final Status removalStatus = remover.remove(path);
+ removeMarkerFile(removalInProgressMarkerFile);
+ if (Status.OK.equals(removalStatus) == false)
+ {
+ // We don't retry this, because the path is local and removal really shouldn't fail.
+ // TODO 2007-09-02, Bernd Rinn: this is no longer true, the source directory can now be remote,
+ // so we should consider retrying a failed attempt to remove it.
+ notificationLog.error(String.format(REMOVING_LOCAL_PATH_FAILED_TEMPLATE, path, removalStatus));
+ } else if (operationLog.isInfoEnabled())
+ {
+ operationLog.info(String.format(REMOVED_PATH_TEMPLATE, path.getPath()));
+ }
+ }
+
+ private boolean isDeletionInProgress(File path)
+ {
+ final File markDeletionInProgressMarkerFile = getDeletionInProgressMarkerFile(path);
+ return markDeletionInProgressMarkerFile.exists();
+ }
+
+ private File tryMarkAsDeletionInProgress(File path)
+ {
+ final File markDeletionInProgressMarkerFile = getDeletionInProgressMarkerFile(path);
+ if (markLocal(markDeletionInProgressMarkerFile))
+ {
+ return markDeletionInProgressMarkerFile;
+ } else
+ {
+ machineLog.error(String.format("Cannot create deletion-in-progress marker file for path '%s' [%s]", path
+ .getAbsolutePath(), markDeletionInProgressMarkerFile.getAbsolutePath()));
+ return null;
+ }
+ }
+
+ private File getDeletionInProgressMarkerFile(File path)
+ {
+ // When destinationHost == null, we put the marker directory in the destination directory, otherwise in
+ // the source directory
+ final File markerParentDirectory = (destinationHost == null) ? destinationDirectory : path.getParentFile();
+ final File markDeletionInProgressMarkerFile =
+ new File(markerParentDirectory, MarkerFile.getDeletionInProgressMarkerName(path));
+ return markDeletionInProgressMarkerFile;
+ }
+
+ private void removeMarkerFile(File markerFileOrNull)
+ {
+ if (markerFileOrNull != null)
+ {
+ final boolean removalOK = markerFileOrNull.delete();
+ if (removalOK == false)
+ {
+ machineLog.error(String.format("Cannot remove marker file '%s'", markerFileOrNull.getAbsolutePath()));
+ }
+ }
+ }
+
private boolean markAsFinished(File path)
{
+ final File markFinishedFile = new File(destinationDirectory, MarkerFile.getCopyFinishedMarkerName(path.getName()));
if (destinationHost == null)
{
- return markAsFinishedLocal(path);
+ return markLocal(markFinishedFile);
} else
{
- return markAsFinishedRemote(path);
+ return markOnSourceLocalAndCopyToRemoteDestination(markFinishedFile);
}
}
- private boolean markAsFinishedLocal(File path)
+ private boolean markLocal(File markerFile)
{
- final File markFile = new File(destinationDirectory, CopyFinishedMarker.getMarkerName(path.getName()));
try
{
- markFile.createNewFile();
- final boolean success = markFile.exists();
+ markerFile.createNewFile();
+ final boolean success = markerFile.exists();
if (success == false)
{
- machineLog.error(String.format(FAILED_TO_CREATE_MARK_FILE_TEMPLATE, markFile.getAbsoluteFile()));
+ machineLog.error(String.format(FAILED_TO_CREATE_MARK_FILE_TEMPLATE, markerFile.getAbsoluteFile()));
}
return success;
} catch (IOException e)
{
- machineLog.error(String.format(FAILED_TO_CREATE_MARK_FILE_TEMPLATE, markFile.getAbsoluteFile()), e);
+ machineLog.error(String.format(FAILED_TO_CREATE_MARK_FILE_TEMPLATE, markerFile.getAbsoluteFile()), e);
return false;
}
}
- private boolean markAsFinishedRemote(File path)
+ private boolean markOnSourceLocalAndCopyToRemoteDestination(File markerFile)
{
- final File markFile = new File(path.getParent(), CopyFinishedMarker.getMarkerName(path.getName()));
+ final File localMarkerFile = new File(markerFile.getParent(), markerFile.getName());
try
{
- markFile.createNewFile();
- monitor.start(path);
- final Status copyStatus = copier.copy(markFile, sourceHost, destinationDirectory, destinationHost);
+ localMarkerFile.createNewFile();
+ monitor.start(localMarkerFile);
+ final Status copyStatus = copier.copy(localMarkerFile, sourceHost, destinationDirectory, destinationHost);
monitor.stop();
if (StatusFlag.OK.equals(copyStatus.getFlag()))
{
return true;
} else
{
- machineLog.error(String.format(FAILED_TO_COPY_MARK_FILE_TO_REMOTE_TEMPLATE, markFile.getAbsoluteFile(),
- copyStatus.toString()));
+ machineLog.error(String.format(FAILED_TO_COPY_MARK_FILE_TO_REMOTE_TEMPLATE, localMarkerFile
+ .getAbsoluteFile(), copyStatus.toString()));
return false;
}
} catch (IOException e)
{
- machineLog.error(String.format(FAILED_TO_CREATE_MARK_FILE_TEMPLATE, markFile.getAbsoluteFile()), e);
+ machineLog.error(String.format(FAILED_TO_CREATE_MARK_FILE_TEMPLATE, localMarkerFile.getAbsoluteFile()), e);
return false;
} finally
{
- markFile.delete();
+ localMarkerFile.delete();
}
}
diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/helper/CopyFinishedMarker.java b/datamover/source/java/ch/systemsx/cisd/datamover/helper/MarkerFile.java
similarity index 53%
rename from datamover/source/java/ch/systemsx/cisd/datamover/helper/CopyFinishedMarker.java
rename to datamover/source/java/ch/systemsx/cisd/datamover/helper/MarkerFile.java
index 11e57163190..28e7076673a 100644
--- a/datamover/source/java/ch/systemsx/cisd/datamover/helper/CopyFinishedMarker.java
+++ b/datamover/source/java/ch/systemsx/cisd/datamover/helper/MarkerFile.java
@@ -22,31 +22,47 @@ import ch.systemsx.cisd.common.Constants;
import ch.systemsx.cisd.common.utilities.FileUtilities;
/**
- * Manipulations on the file which is used as a marker of finished copy operation.
+ * Manipulations on marker files.
*
* @author Tomasz Pylak on Aug 27, 2007
*/
-public class CopyFinishedMarker
+public class MarkerFile
{
- public static String getMarkerName(String originalFileName)
+ public static String getCopyFinishedMarkerName(String originalFileName)
{
return Constants.IS_FINISHED_PREFIX + originalFileName;
}
- public static boolean isMarker(File file)
+ public static String getDeletionInProgressMarkerName(File originalFile)
+ {
+ return Constants.DELETION_IN_PROGRESS_PREFIX + originalFile.getName();
+ }
+
+ public static String getDeletionInProgressMarkerName(String originalFileName)
+ {
+ return Constants.DELETION_IN_PROGRESS_PREFIX + originalFileName;
+ }
+
+ public static boolean isCopyFinishedMarker(File file)
{
return file.getName().startsWith(Constants.IS_FINISHED_PREFIX);
}
- public static File extractOriginal(File markerFile)
+ public static boolean isDeletionInProgressMarker(File file)
{
- assert isMarker(markerFile);
+ return file.getName().startsWith(Constants.DELETION_IN_PROGRESS_PREFIX);
+ }
+
+ public static File extractOriginalFromCopyFinishedMarker(File markerFile)
+ {
+ assert isCopyFinishedMarker(markerFile);
return FileUtilities.removePrefixFromFileName(markerFile, Constants.IS_FINISHED_PREFIX);
}
- public static File extractMarker(File originalFile)
+ public static File createCopyFinishedMarker(File originalFile)
{
- return new File(originalFile.getParent(), getMarkerName(originalFile.getName()));
+ return new File(originalFile.getParent(), getCopyFinishedMarkerName(originalFile.getName()));
}
+
}
diff --git a/datamover/sourceTest/java/ch/systemsx/cisd/datamover/MainTest.java b/datamover/sourceTest/java/ch/systemsx/cisd/datamover/MainTest.java
index d1c167d2eea..71c267c1a98 100644
--- a/datamover/sourceTest/java/ch/systemsx/cisd/datamover/MainTest.java
+++ b/datamover/sourceTest/java/ch/systemsx/cisd/datamover/MainTest.java
@@ -63,7 +63,7 @@ public class MainTest
assert unitTestRootDirectory.isDirectory();
}
- @BeforeMethod
+ @BeforeMethod(alwaysRun=true)
public void setUp()
{
FileUtilities.deleteRecursively(workingDirectory);
@@ -173,9 +173,14 @@ public class MainTest
DEFAULT_STRUCT.createPartialSampleStructure(parentDir);
}
- private static void createSampleMarkerFile(File parentDir)
+ private static void createSampleFinishedMarkerFile(File parentDir)
{
- DEFAULT_STRUCT.createSampleMarkerFile(parentDir);
+ DEFAULT_STRUCT.createSampleFinishedMarkerFile(parentDir);
+ }
+
+ private static void createSampleDeletionInProgressMarkerFile(File parentDir)
+ {
+ DEFAULT_STRUCT.createSampleDeletionInProgressMarkerFile(parentDir);
}
private static void assertSampleStructureExists(File parentDir) throws IOException
@@ -345,7 +350,7 @@ public class MainTest
public void prepareState(ExternalDirs dirs, LocalBufferDirs bufferDirs) throws Exception
{
createSampleStructure(bufferDirs.getCopyInProgressDir());
- createSampleMarkerFile(bufferDirs.getCopyInProgressDir());
+ createSampleFinishedMarkerFile(bufferDirs.getCopyInProgressDir());
}
});
}
@@ -359,8 +364,24 @@ public class MainTest
{
public void prepareState(ExternalDirs dirs, LocalBufferDirs bufferDirs) throws Exception
{
- createSampleStructure(bufferDirs.getCopyCompleteDir());
- createSampleMarkerFile(bufferDirs.getCopyInProgressDir());
+ createSampleStructure(bufferDirs.getCopyInProgressDir());
+ createSampleFinishedMarkerFile(bufferDirs.getCopyInProgressDir());
+ }
+ });
+ }
+
+ @Test(groups =
+ { "slow" })
+ // recovery after failure when data are copied to 'copy-completed', but before deletion has been finished
+ public void testRecoveryIncomingCompleteDeletionInProgress() throws Exception
+ {
+ performGenericTest(new IFSPreparator()
+ {
+ public void prepareState(ExternalDirs dirs, LocalBufferDirs bufferDirs) throws Exception
+ {
+ createSampleStructure(bufferDirs.getCopyInProgressDir());
+ createPartialSampleStructure(dirs.incoming);
+ createSampleDeletionInProgressMarkerFile(bufferDirs.getCopyInProgressDir());
}
});
}
diff --git a/datamover/sourceTest/java/ch/systemsx/cisd/datamover/testhelper/FileStructEngine.java b/datamover/sourceTest/java/ch/systemsx/cisd/datamover/testhelper/FileStructEngine.java
index 47c147b1685..5a579a2f155 100644
--- a/datamover/sourceTest/java/ch/systemsx/cisd/datamover/testhelper/FileStructEngine.java
+++ b/datamover/sourceTest/java/ch/systemsx/cisd/datamover/testhelper/FileStructEngine.java
@@ -23,11 +23,12 @@ import java.util.Arrays;
import java.util.List;
import static ch.systemsx.cisd.datamover.testhelper.FileSystemHelper.*;
+
import ch.systemsx.cisd.common.utilities.CollectionIO;
-import ch.systemsx.cisd.datamover.helper.CopyFinishedMarker;
+import ch.systemsx.cisd.datamover.helper.MarkerFile;
/**
- * Immutable helper for creating a sample directory structure and manipulating on it.
+ * Immutable helper for creating a sample directory structure and manipulating it.
*
* @author Tomasz Pylak on Aug 29, 2007
*/
@@ -73,16 +74,26 @@ public class FileStructEngine
createSampleFile(dir1, SAMPLE_FILE1);
}
- public void createSampleMarkerFile(File parentDir)
+ public void createSampleFinishedMarkerFile(File parentDir)
{
- createEmptyFile(createMarkerFile(parentDir, sampleMovedDir));
+ createEmptyFile(createFinishedMarkerFile(parentDir, sampleMovedDir));
}
- private static File createMarkerFile(File parentDir, String originalName)
+ public void createSampleDeletionInProgressMarkerFile(File parentDir)
{
- return new File(parentDir, CopyFinishedMarker.getMarkerName(originalName));
+ createEmptyFile(createDeletionInProgressMarkerFile(parentDir, sampleMovedDir));
}
+ private static File createFinishedMarkerFile(File parentDir, String originalName)
+ {
+ return new File(parentDir, MarkerFile.getCopyFinishedMarkerName(originalName));
+ }
+
+ private static File createDeletionInProgressMarkerFile(File parentDir, String originalName)
+ {
+ return new File(parentDir, MarkerFile.getDeletionInProgressMarkerName(originalName));
+ }
+
private static List<String> createSampleFileContent()
{
String[] lines = new String[]
@@ -114,7 +125,7 @@ public class FileStructEngine
public void assertSampleStructFinishMarkerExists(File parentDir)
{
- File marker = createMarkerFile(parentDir, sampleMovedDir);
+ File marker = createFinishedMarkerFile(parentDir, sampleMovedDir);
assert marker.exists();
}
}
--
GitLab