diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/AbstractOmniscientTopLevelDataSetRegistrator.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/AbstractOmniscientTopLevelDataSetRegistrator.java index 8fc4b2ec2a09ebac165b0298282c1e1adc7e124d..083f5ac4dbd2dcac46eb0a20e80e21fc50d2007a 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/AbstractOmniscientTopLevelDataSetRegistrator.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/AbstractOmniscientTopLevelDataSetRegistrator.java @@ -301,19 +301,10 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat return getGlobalState().getDssInternalTempDir(); } - /** - * returns the recovery marker file if found, or null otherwise. It first checks if the incoming - * is the marker file, then if there is a marker file corresponding to this incoming file - */ - private boolean isRecoveryMarkerFile(File incoming) - { - return state.getGlobalState().getStorageRecoveryManager().isRecoveryFile(incoming); - } - protected boolean hasRecoveryMarkerFile(File incoming) { - return new File(incoming.getAbsolutePath() - + IDataSetStorageRecoveryManager.PROCESSING_MARKER).exists(); + return getGlobalState().getStorageRecoveryManager().getProcessingMarkerFile(incoming) + .exists(); } /** @@ -335,21 +326,11 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat .getIncomingDataSetPathFromMarker(incomingDataSetFileOrIsFinishedFile) : incomingDataSetFileOrIsFinishedFile; - if (isRecoveryMarkerFile(incomingDataSetFile)) + if (hasRecoveryMarkerFile(incomingDataSetFile)) { handleRecovery(incomingDataSetFile); - return; - } else if (hasRecoveryMarkerFile(incomingDataSetFile)) - { - operationLog.info("Ignore file, as the recovery marker exists for it: " - + incomingDataSetFile.getAbsolutePath()); // will handle only the recovery file - don't do anything return; - } else if (false == incomingDataSetFile.exists()) - { - operationLog.info("The file doesn't exist: " + incomingDataSetFile.getAbsolutePath()); - // it can mean that the recovery has already cleaned this file - return; } final IDelegatedActionWithResult<Boolean> markerFileCleanupAction; @@ -487,14 +468,18 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat return new RuntimeException(cause.toString()); } - private void handleRecovery(final File recoveryMarkerFile) + private void handleRecovery(final File incomingFileOriginal) { + //get the marker file + final File recoveryMarkerFile = + state.getGlobalState().getStorageRecoveryManager() + .getProcessingMarkerFile(incomingFileOriginal); + + //deserialize recovery state final DataSetStoragePrecommitRecoveryState<T> recoveryState = state.getGlobalState().getStorageRecoveryManager() .extractPrecommittedCheckpoint(recoveryMarkerFile); - // TODO: cleanup also with the incoming marker file, or are we guaranteed that the marker - // file is gone at this step. // then we should ensure that the recovery will actually take place itself! final File recoveryFile = state.getGlobalState().getStorageRecoveryManager() diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRecoveryManager.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRecoveryManager.java index 2bf89ae0cc3aea35c9e467c7862a99902990d268..12e4005d14142a7ca5f2741158e79f6f74052b0b 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRecoveryManager.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRecoveryManager.java @@ -33,6 +33,10 @@ import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetInformation; */ public class DataSetStorageRecoveryManager implements IDataSetStorageRecoveryManager { + private static final String PRECOMMIT_SERIALIZED = ".PRECOMMIT_SERIALIZED"; + + private static final String PROCESSING_MARKER = ".PROCESSING_MARKER"; + static final Logger operationLog = LogFactory.getLogger(LogCategory.OPERATION, DataSetStorageRecoveryManager.class); @@ -69,10 +73,17 @@ public class DataSetStorageRecoveryManager implements IDataSetStorageRecoveryMan private <T extends DataSetInformation> File getProcessingMarkerFile( DataSetStorageAlgorithmRunner<T> runner) { - return new File(runner.getIncomingDataSetFile().getRealIncomingFile().getParentFile(), - runner.getIncomingDataSetFile().getRealIncomingFile().getName() + PROCESSING_MARKER); + return getProcessingMarkerFile(runner.getIncomingDataSetFile().getRealIncomingFile()); } + /** + * @return processing marker file for a given incoming file. + */ + public File getProcessingMarkerFile(File incoming) + { + return new File(incoming.getParentFile(), incoming.getName() + PROCESSING_MARKER); + } + private <T extends DataSetInformation> File getSerializedFile( DataSetStorageAlgorithmRunner<T> runner) { @@ -125,7 +136,7 @@ public class DataSetStorageRecoveryManager implements IDataSetStorageRecoveryMan return true; } - public boolean isRecoveryFile(File file) + public static boolean isRecoveryFile(File file) { return file.getName().endsWith(PROCESSING_MARKER); } diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IDataSetStorageRecoveryManager.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IDataSetStorageRecoveryManager.java index 425813a15c4724e9ddf4f4bce48fb528a045a610..25a55409e69bbf0b5b022048df3e633249a29874 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IDataSetStorageRecoveryManager.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IDataSetStorageRecoveryManager.java @@ -27,9 +27,6 @@ import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetInformation; */ public interface IDataSetStorageRecoveryManager { - public static final String PRECOMMIT_SERIALIZED = ".PRECOMMIT_SERIALIZED"; - - public static final String PROCESSING_MARKER = ".PROCESSING_MARKER"; // Recovery Mechanics /** @@ -62,13 +59,13 @@ public interface IDataSetStorageRecoveryManager */ File getRecoveryFileFromMarker(File markerFile); - // Simple helper methods - boolean canRecoverFromError(Throwable ex); - /** - * checks whether the file is a proper recovery marker file + * @return the path of the recovery marker file for the given incoming */ - boolean isRecoveryFile(File file); + File getProcessingMarkerFile(File incoming); + + // Simple helper methods + boolean canRecoverFromError(Throwable ex); void setDropboxRecoveryStateDir(File dropboxRecoveryStateDir); } \ No newline at end of file diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonDropboxRecoveryTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonDropboxRecoveryTest.java index 2683a3e76ded72837466d51db8b3e005de90d4bd..647037ca3e504a95a57dc5df20805dcfb9754a16 100644 --- a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonDropboxRecoveryTest.java +++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonDropboxRecoveryTest.java @@ -148,19 +148,19 @@ public class JythonDropboxRecoveryTest extends AbstractJythonDataSetHandlerTest if (testCase.canRecoverFromError) { - File recoveryMarkerFile = assertRecoveryMarkerFile(); + assertRecoveryMarkerFile(); assertOriginalMarkerFileExists(); - handler.handle(recoveryMarkerFile); + handler.handle(markerFile); // if failure happened here then don't expect recovery / marker files to be deleted if (testCase.registrationSuccessful) { - //item in store + // item in store assertStorageProcess(atomicatOperationDetails.recordedObject(), DATA_SET_CODE, "sub_data_set_1", 0); - //FIXME: this is commented out to cover the bug! beware + // FIXME: this is commented out to cover the bug! beware // assertDirEmpty(stagingDirectory); assertDirEmpty(precommitDirectory); } else @@ -188,10 +188,10 @@ public class JythonDropboxRecoveryTest extends AbstractJythonDataSetHandlerTest // we continue with the recovery } - + private void assertDirEmpty(File file) { - String contents = file.getAbsolutePath(); + String contents = file.getAbsolutePath(); assertEquals(contents, "[]", Arrays.asList(file.list()).toString()); } @@ -225,8 +225,8 @@ public class JythonDropboxRecoveryTest extends AbstractJythonDataSetHandlerTest File originalIncoming = FileUtilities.removePrefixFromFileName(markerFile, IS_FINISHED_PREFIX); File recoveryMarkerFile = - new File(originalIncoming.getAbsolutePath() - + IDataSetStorageRecoveryManager.PROCESSING_MARKER); + handler.getGlobalState().getStorageRecoveryManager() + .getProcessingMarkerFile(originalIncoming); return recoveryMarkerFile; } diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorTest.java index 63945836829471c7442ec81094d76c18b7a82c6b..5999afd1f66bd5d58f6464463a5a2086236610ef 100644 --- a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorTest.java +++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorTest.java @@ -535,8 +535,8 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH protected void setupExpectations() { - checkIfRecoveryFile(); - + generalAllowing(); + if (testCase.failurePoint != null && testCase.failurePoint .compareTo(TestCaseParameters.FailurePoint.DURING_OPENBIS_REGISTRATION) < 0) @@ -592,13 +592,12 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH will(checkPrecommitDirIsEmpty()); } - protected void checkIfRecoveryFile() + private void generalAllowing() { - // unless this is a recovery testcase - one(storageRecoveryManager).isRecoveryFile(with(any(File.class))); - will(returnValue(false)); + allowing(storageRecoveryManager).getProcessingMarkerFile(with(any(File.class))); + will(returnValue(new File(incomingDataSetFile.getAbsolutePath()+".NON_EXISTING"))); } - + @SuppressWarnings("unchecked") private void cleanRecoveryCheckpoint(boolean required) {