From 3ec625dc3f80e5580a763e715e28706d40fce7af Mon Sep 17 00:00:00 2001 From: cramakri <cramakri> Date: Mon, 23 May 2011 10:48:26 +0000 Subject: [PATCH] LMS-2196 Changes for new rollback infrastructure. SVN: 21425 --- .../cisd/etlserver/ThreadParameters.java | 24 ++++++ ...tOmniscientTopLevelDataSetRegistrator.java | 53 ++++++++++++- .../ContainerDataSetStorageAlgorithm.java | 2 +- .../DataSetRegistrationService.java | 63 +++++++-------- .../registrator/DataSetStorageAlgorithm.java | 64 +-------------- .../DataSetStorageAlgorithmRunner.java | 28 ++++--- .../registrator/DataSetStorageRollbacker.java | 8 +- .../JythonTopLevelDataSetHandler.java | 4 +- .../impl/DataSetRegistrationTransaction.java | 6 +- .../ConfiguredOnErrorActionDecisionTest.java | 79 +++++++++++++++++++ .../DataSetStorageRollbackerTest.java | 44 +---------- .../JythonTopLevelDataSetRegistratorTest.java | 4 +- 12 files changed, 216 insertions(+), 163 deletions(-) create mode 100644 datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/ConfiguredOnErrorActionDecisionTest.java diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/ThreadParameters.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/ThreadParameters.java index eba8b7f1cbc..3dbfd2881a3 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/ThreadParameters.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/ThreadParameters.java @@ -90,6 +90,9 @@ public final class ThreadParameters private static final String REPROCESS_FAULTY_DATASETS_NAME = "reprocess-faulty-datasets"; + @Private + public static final String ON_ERROR_DECISION_KEY = "on-error-decision"; + /** * The (local) directory to monitor for new files and directories to move to the remote side. * The directory where data to be processed by the ETL server become available. @@ -102,6 +105,8 @@ public final class ThreadParameters private final Class<?> topLevelDataSetRegistratorClassOrNull; + private final Class<?> onErrorDecisionClassOrNull; + private final String threadName; private final String groupCode; @@ -158,6 +163,20 @@ public final class ThreadParameters "false")); this.threadName = threadName; + + String onErrorClassName = + PropertyUtils.getProperty(threadProperties, ON_ERROR_DECISION_KEY + ".class"); + Class<?> onErrorClass; + try + { + onErrorClass = (null == onErrorClassName) ? null : Class.forName(onErrorClassName); + } catch (ClassNotFoundException ex) + { + throw ConfigurationFailureException.fromTemplate("Wrong '%s' property: %s", + ON_ERROR_DECISION_KEY + ".class", ex.getMessage()); + } + this.onErrorDecisionClassOrNull = onErrorClass; + } // true if marker file should be used, false if autodetection should be used, exceprion when the @@ -280,6 +299,11 @@ public final class ThreadParameters : topLevelDataSetRegistratorClassOrNull; } + public Class<?> getOnErrorActionDecisionClass(Class<?> defaultClass) + { + return (onErrorDecisionClassOrNull == null) ? defaultClass : onErrorDecisionClassOrNull; + } + public Properties getThreadProperties() { return threadProperties; 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 7dd5b57a6f3..14304a6f1d1 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 @@ -17,9 +17,11 @@ package ch.systemsx.cisd.etlserver.registrator; import static ch.systemsx.cisd.etlserver.IStorageProcessorTransactional.STORAGE_PROCESSOR_KEY; +import static ch.systemsx.cisd.etlserver.ThreadParameters.ON_ERROR_DECISION_KEY; import java.io.File; import java.util.List; +import java.util.Properties; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -32,6 +34,8 @@ import ch.systemsx.cisd.common.filesystem.FileOperations; import ch.systemsx.cisd.common.filesystem.IFileOperations; import ch.systemsx.cisd.common.logging.LogCategory; import ch.systemsx.cisd.common.logging.LogFactory; +import ch.systemsx.cisd.common.utilities.ClassUtils; +import ch.systemsx.cisd.common.utilities.ExtendedProperties; import ch.systemsx.cisd.common.utilities.IDelegatedActionWithResult; import ch.systemsx.cisd.etlserver.AbstractTopLevelDataSetRegistrator; import ch.systemsx.cisd.etlserver.DataStrategyStore; @@ -39,9 +43,11 @@ import ch.systemsx.cisd.etlserver.IDataStrategyStore; import ch.systemsx.cisd.etlserver.IPostRegistrationAction; import ch.systemsx.cisd.etlserver.IPreRegistrationAction; import ch.systemsx.cisd.etlserver.IStorageProcessorTransactional; +import ch.systemsx.cisd.etlserver.IStorageProcessorTransactional.UnstoreDataAction; import ch.systemsx.cisd.etlserver.ITopLevelDataSetRegistratorDelegate; import ch.systemsx.cisd.etlserver.PropertiesBasedETLServerPlugin; import ch.systemsx.cisd.etlserver.TopLevelDataSetRegistratorGlobalState; +import ch.systemsx.cisd.etlserver.registrator.IDataSetOnErrorActionDecision.ErrorType; import ch.systemsx.cisd.etlserver.registrator.api.v1.impl.DataSetRegistrationTransaction; import ch.systemsx.cisd.etlserver.utils.PostRegistrationExecutor; import ch.systemsx.cisd.etlserver.utils.PreRegistrationExecutor; @@ -93,10 +99,12 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat private final ValidationScriptRunner validationScriptRunner; + private final IDataSetOnErrorActionDecision onErrorActionDecision; + private OmniscientTopLevelDataSetRegistratorState( TopLevelDataSetRegistratorGlobalState globalState, IStorageProcessorTransactional storageProcessor, ReentrantLock registrationLock, - IFileOperations fileOperations) + IFileOperations fileOperations, IDataSetOnErrorActionDecision onErrorActionDecision) { this.globalState = globalState; this.storageProcessor = storageProcessor; @@ -116,6 +124,7 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat this.validationScriptRunner = ValidationScriptRunner.createValidatorFromScriptPath(globalState .getValidationScriptOrNull()); + this.onErrorActionDecision = onErrorActionDecision; } public TopLevelDataSetRegistratorGlobalState getGlobalState() @@ -167,6 +176,11 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat { return validationScriptRunner; } + + public IDataSetOnErrorActionDecision getOnErrorActionDecision() + { + return onErrorActionDecision; + } } public static class DoNothingDelegatedAction implements IDelegatedActionWithResult<Boolean> @@ -206,9 +220,19 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat STORAGE_PROCESSOR_KEY, true); storageProcessor.setStoreRootDirectory(globalState.getStoreRootDir()); + Properties onErrorDecisionProperties = + ExtendedProperties.getSubset(globalState.getThreadParameters() + .getThreadProperties(), ON_ERROR_DECISION_KEY, true); + IDataSetOnErrorActionDecision onErrorDecision = + ClassUtils.create( + IDataSetOnErrorActionDecision.class, + globalState.getThreadParameters().getOnErrorActionDecisionClass( + ConfiguredOnErrorActionDecision.class), onErrorDecisionProperties); + state = new OmniscientTopLevelDataSetRegistratorState(globalState, storageProcessor, - new ReentrantLock(), FileOperations.getMonitoredInstanceForCurrentThread()); + new ReentrantLock(), FileOperations.getMonitoredInstanceForCurrentThread(), + onErrorDecision); DataSetRegistrationTransaction.rollbackDeadTransactions(globalState.getStoreRootDir()); @@ -316,6 +340,18 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat } catch (Throwable ex) { operationLog.error("Could not process file " + incomingDataSetFile, ex); + + // If we are here, it is because there was an error thrown in Java before trying to + // register the data set. This is considered a script error + UnstoreDataAction action = + getRegistratorState().getOnErrorActionDecision().computeUndoAction( + ErrorType.REGISTRATION_SCRIPT_ERROR, ex); + DataSetStorageRollbacker rollbacker = + new DataSetStorageRollbacker(getRegistratorState(), operationLog, action, + incomingDataSetFile, null, ex, ErrorType.REGISTRATION_SCRIPT_ERROR); + operationLog.info(rollbacker.getErrorMessageForLog()); + rollbacker.doRollback(); + rollback(service, ex); } @@ -341,7 +377,16 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat sb.append(error.getErrorMessage()); sb.append("\n"); } - operationLog.error(sb.toString()); + + UnstoreDataAction action = + getRegistratorState().getOnErrorActionDecision().computeUndoAction( + ErrorType.INVALID_DATA_SET, null); + DataSetStorageRollbacker rollbacker = + new DataSetStorageRollbacker(getRegistratorState(), operationLog, action, + incomingDataSetFile, null, null, ErrorType.INVALID_DATA_SET); + sb.append(rollbacker.getErrorMessageForLog()); + operationLog.info(sb.toString()); + rollbacker.doRollback(); } public boolean isStopped() @@ -369,7 +414,7 @@ public abstract class AbstractOmniscientTopLevelDataSetRegistrator<T extends Dat * Subclasses may override, but should call super. */ - public void rollbackTransaction(DataSetRegistrationService<T> dataSetRegistrationService, + public void didRollbackTransaction(DataSetRegistrationService<T> dataSetRegistrationService, DataSetRegistrationTransaction<T> transaction, DataSetStorageAlgorithmRunner<T> algorithm, Throwable ex) { diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/ContainerDataSetStorageAlgorithm.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/ContainerDataSetStorageAlgorithm.java index 6deb15fe495..f7c91bec843 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/ContainerDataSetStorageAlgorithm.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/ContainerDataSetStorageAlgorithm.java @@ -195,7 +195,7 @@ public class ContainerDataSetStorageAlgorithm<T extends DataSetInformation> exte } /** - * Committed data sets don't use a storage process -- there is nothing to do. + * Committed data sets don't use a storage processor -- there is nothing to do. */ public UnstoreDataAction rollbackStorageProcessor(final Throwable throwable) { diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetRegistrationService.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetRegistrationService.java index 981dcc7be72..f5401bb585f 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetRegistrationService.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetRegistrationService.java @@ -21,23 +21,25 @@ import java.util.ArrayList; import java.util.List; import java.util.Properties; +import org.apache.log4j.Logger; + +import ch.systemsx.cisd.base.exceptions.InterruptedExceptionUnchecked; +import ch.systemsx.cisd.common.logging.LogCategory; +import ch.systemsx.cisd.common.logging.LogFactory; import ch.systemsx.cisd.common.utilities.IDelegatedActionWithResult; import ch.systemsx.cisd.common.utilities.PropertyUtils; -import ch.systemsx.cisd.etlserver.BaseDirectoryHolder; import ch.systemsx.cisd.etlserver.DataSetRegistrationAlgorithm; import ch.systemsx.cisd.etlserver.DataSetRegistrationAlgorithmRunner; -import ch.systemsx.cisd.etlserver.FileRenamer; import ch.systemsx.cisd.etlserver.IDataStoreStrategy; +import ch.systemsx.cisd.etlserver.IStorageProcessorTransactional.UnstoreDataAction; import ch.systemsx.cisd.etlserver.ITopLevelDataSetRegistratorDelegate; import ch.systemsx.cisd.etlserver.IdentifiedDataStrategy; import ch.systemsx.cisd.etlserver.TopLevelDataSetRegistratorGlobalState; -import ch.systemsx.cisd.etlserver.TransferredDataSetHandler; import ch.systemsx.cisd.etlserver.registrator.AbstractOmniscientTopLevelDataSetRegistrator.OmniscientTopLevelDataSetRegistratorState; +import ch.systemsx.cisd.etlserver.registrator.IDataSetOnErrorActionDecision.ErrorType; import ch.systemsx.cisd.etlserver.registrator.api.v1.IDataSetRegistrationTransaction; import ch.systemsx.cisd.etlserver.registrator.api.v1.impl.DataSetRegistrationTransaction; import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetInformation; -import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetType; -import ch.systemsx.cisd.openbis.generic.shared.dto.types.DataSetTypeCode; /** * A service that registers many files as individual data sets in one transaction. @@ -66,6 +68,9 @@ public class DataSetRegistrationService<T extends DataSetInformation> implements private final ITopLevelDataSetRegistratorDelegate delegate; + static private final Logger operationLog = LogFactory.getLogger(LogCategory.OPERATION, + DataSetRegistrationService.class); + /** * Keep track of errors we encounter while processing. Clients may want this information. */ @@ -174,38 +179,30 @@ public class DataSetRegistrationService<T extends DataSetInformation> implements public File moveIncomingToError(String dataSetTypeCodeOrNull) { - // Make sure the data set information is valid - DataSetInformation dataSetInfo = new DataSetInformation(); - dataSetInfo.setShareId(registratorContext.getGlobalState().getShareId()); - if (null == dataSetTypeCodeOrNull) - { - dataSetInfo.setDataSetType(new DataSetType(DataSetTypeCode.UNKNOWN.getCode())); - } else - { - dataSetInfo.setDataSetType(new DataSetType(dataSetTypeCodeOrNull)); - } - - // Create the error directory - File baseDirectory = - DataSetStorageAlgorithm.createBaseDirectory( - TransferredDataSetHandler.ERROR_DATA_STRATEGY, registratorContext - .getStorageProcessor().getStoreRootDirectory(), registratorContext - .getFileOperations(), dataSetInfo, dataSetInfo.getDataSetType(), - incomingDataSetFile); - BaseDirectoryHolder baseDirectoryHolder = - new BaseDirectoryHolder(TransferredDataSetHandler.ERROR_DATA_STRATEGY, - baseDirectory, incomingDataSetFile); - - // Move the incoming there - FileRenamer.renameAndLog(incomingDataSetFile, baseDirectoryHolder.getTargetFile()); - return baseDirectoryHolder.getTargetFile(); + DataSetStorageRollbacker rollbacker = + new DataSetStorageRollbacker(registratorContext, operationLog, + UnstoreDataAction.MOVE_TO_ERROR, incomingDataSetFile, + dataSetTypeCodeOrNull, null); + return rollbacker.doRollback(); } - public void rollbackTransaction(DataSetRegistrationTransaction<T> transaction, - DataSetStorageAlgorithmRunner<T> algorithm, Throwable ex) + public void didRollbackTransaction(DataSetRegistrationTransaction<T> transaction, + DataSetStorageAlgorithmRunner<T> algorithm, Throwable ex, ErrorType errorType) { encounteredErrors.add(ex); - registrator.rollbackTransaction(this, transaction, algorithm, ex); + // Don't do the undo store action when this exception happens + boolean stopped = ex instanceof InterruptedExceptionUnchecked; + if (false == stopped) + { + UnstoreDataAction action = + registratorContext.getOnErrorActionDecision().computeUndoAction(errorType, ex); + DataSetStorageRollbacker rollbacker = + new DataSetStorageRollbacker(registratorContext, operationLog, action, + incomingDataSetFile, null, null, errorType); + operationLog.info(rollbacker.getErrorMessageForLog()); + rollbacker.doRollback(); + } + registrator.didRollbackTransaction(this, transaction, algorithm, ex); } /** diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithm.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithm.java index 31db8b6fbaa..6c0eecada42 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithm.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithm.java @@ -17,11 +17,7 @@ package ch.systemsx.cisd.etlserver.registrator; import java.io.File; -import java.io.FileWriter; -import java.io.IOException; -import java.io.PrintWriter; -import org.apache.commons.io.IOUtils; import org.apache.commons.lang.time.StopWatch; import org.apache.log4j.Logger; @@ -30,13 +26,11 @@ import ch.systemsx.cisd.common.Constants; import ch.systemsx.cisd.common.exceptions.EnvironmentFailureException; import ch.systemsx.cisd.common.filesystem.FileUtilities; import ch.systemsx.cisd.common.filesystem.IFileOperations; -import ch.systemsx.cisd.common.logging.Log4jSimpleLogger; import ch.systemsx.cisd.common.logging.LogCategory; import ch.systemsx.cisd.common.logging.LogFactory; import ch.systemsx.cisd.common.mail.IMailClient; import ch.systemsx.cisd.etlserver.BaseDirectoryHolder; import ch.systemsx.cisd.etlserver.DataStoreStrategyKey; -import ch.systemsx.cisd.etlserver.FileRenamer; import ch.systemsx.cisd.etlserver.IDataStoreStrategy; import ch.systemsx.cisd.etlserver.IStorageProcessorTransactional; import ch.systemsx.cisd.etlserver.IStorageProcessorTransactional.IStorageProcessorTransaction; @@ -202,7 +196,10 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation> } RolledbackState<T> rolledbackState = (RolledbackState<T>) state; - rolledbackState.executeUndoAction(); + + // Do not undo the individual data sets -- the entire transaction needs to be undone as a + // group + // rolledbackState.executeUndoAction(); state = new UndoneState<T>(rolledbackState); } @@ -429,8 +426,6 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation> private static class StoredState<T extends DataSetInformation> extends DataSetStorageAlgorithmState<T> { - protected final IStorageProcessorTransactional storageProcessor; - protected final IStorageProcessorTransaction transaction; protected final File markerFile; @@ -438,7 +433,6 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation> public StoredState(PreparedState<T> oldState) { super(oldState.storageAlgorithm); - this.storageProcessor = storageAlgorithm.getStorageProcessor(); this.transaction = oldState.transaction; this.markerFile = oldState.markerFile; } @@ -493,61 +487,11 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation> private static class RolledbackState<T extends DataSetInformation> extends DataSetStorageAlgorithmState<T> { - private final UnstoreDataAction action; - - private final Throwable throwable; - - private final File storeRoot; - - private BaseDirectoryHolder baseDirectoryHolder; public RolledbackState(StoredState<T> oldState, UnstoreDataAction action, Throwable throwable) { super(oldState.storageAlgorithm); - this.action = action; - this.throwable = throwable; - this.storeRoot = oldState.storageProcessor.getStoreRootDirectory(); - } - - public void executeUndoAction() - { - if (action == UnstoreDataAction.MOVE_TO_ERROR) - { - final File baseDirectory = - createBaseDirectory(TransferredDataSetHandler.ERROR_DATA_STRATEGY, - storeRoot, storageAlgorithm.getDataSetInformation()); - baseDirectoryHolder = - new BaseDirectoryHolder(TransferredDataSetHandler.ERROR_DATA_STRATEGY, - baseDirectory, incomingDataSetFile); - FileRenamer.renameAndLog(incomingDataSetFile, baseDirectoryHolder.getTargetFile()); - writeThrowable(); - } else if (action == UnstoreDataAction.DELETE) - { - FileUtilities.deleteRecursively(incomingDataSetFile, new Log4jSimpleLogger( - getOperationLog())); - } - } - - private void writeThrowable() - { - final String fileName = incomingDataSetFile.getName() + ".exception"; - final File file = - new File(baseDirectoryHolder.getTargetFile().getParentFile(), fileName); - FileWriter writer = null; - try - { - writer = new FileWriter(file); - throwable.printStackTrace(new PrintWriter(writer)); - } catch (final IOException e) - { - getOperationLog().warn( - String.format("Could not write out the exception '%s' in file '%s'.", - fileName, file.getAbsolutePath()), e); - } finally - { - IOUtils.closeQuietly(writer); - } } } diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithmRunner.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithmRunner.java index be85cc51c45..45743947082 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithmRunner.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithmRunner.java @@ -22,9 +22,9 @@ import java.util.List; import org.apache.log4j.Logger; -import ch.systemsx.cisd.base.exceptions.InterruptedExceptionUnchecked; import ch.systemsx.cisd.common.logging.LogCategory; import ch.systemsx.cisd.common.logging.LogFactory; +import ch.systemsx.cisd.etlserver.registrator.IDataSetOnErrorActionDecision.ErrorType; import ch.systemsx.cisd.openbis.dss.generic.shared.IEncapsulatedOpenBISService; import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetInformation; import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetRegistrationInformation; @@ -40,7 +40,13 @@ public class DataSetStorageAlgorithmRunner<T extends DataSetInformation> public static interface IRollbackDelegate<T extends DataSetInformation> { - public void rollback(DataSetStorageAlgorithmRunner<T> algorithm, Throwable ex); + /** + * @param algorithm The algorithm that is rolling back + * @param ex The throwable that forced the rollback + * @param errorType The point in the execution of the algorithm that rollback happened + */ + public void didRollbackStorageAlgorithmRunner(DataSetStorageAlgorithmRunner<T> algorithm, + Throwable ex, ErrorType errorType); } /** @@ -160,21 +166,24 @@ public class DataSetStorageAlgorithmRunner<T extends DataSetInformation> { operationLog.error("Failed to run storage processor", ex); rollbackStorageProcessors(ex); - rollbackDelegate.rollback(this, ex); + rollbackDelegate.didRollbackStorageAlgorithmRunner(this, ex, + ErrorType.STORAGE_PROCESSOR_ERROR); } private void rollbackDuringMetadataRegistration(Throwable ex) { operationLog.error("Failed to register metadata", ex); rollbackStorageProcessors(ex); - rollbackDelegate.rollback(this, ex); + rollbackDelegate.didRollbackStorageAlgorithmRunner(this, ex, + ErrorType.OPENBIS_REGISTRATION_FAILURE); } private void rollbackAfterStorageProcessorAndMetadataRegistration(Throwable ex) { operationLog.error("Failed to complete transaction", ex); rollbackStorageProcessors(ex); - rollbackDelegate.rollback(this, ex); + rollbackDelegate.didRollbackStorageAlgorithmRunner(this, ex, + ErrorType.POST_REGISTRATION_ERROR); } private void commitStorageProcessors() @@ -219,19 +228,12 @@ public class DataSetStorageAlgorithmRunner<T extends DataSetInformation> return; } - // Don't rollback when this exception happens - boolean stopped = ex instanceof InterruptedExceptionUnchecked; - // Rollback in the reverse order for (int i = dataSetStorageAlgorithms.size() - 1; i >= 0; --i) { DataSetStorageAlgorithm<T> storageAlgorithm = dataSetStorageAlgorithms.get(i); storageAlgorithm.rollbackStorageProcessor(ex); - - if (stopped == false) - { - storageAlgorithm.executeUndoStoreAction(); - } + storageAlgorithm.executeUndoStoreAction(); } } diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRollbacker.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRollbacker.java index 244b3353ec9..73481eff18e 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRollbacker.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRollbacker.java @@ -101,15 +101,15 @@ public class DataSetStorageRollbacker this.errorTypeOrNull = errorTypeOrNull; } - public void appendErrorMessage() + public String getErrorMessageForLog() { if (errorTypeOrNull != null) { - operationLog.info("Responding to error [" + errorTypeOrNull + "] by performing action " - + unstoreAction + " on " + incomingDataSetFile); + return "Responding to error [" + errorTypeOrNull + "] by performing action " + + unstoreAction + " on " + incomingDataSetFile; } else { - operationLog.info("Performing action " + unstoreAction + " on " + incomingDataSetFile); + return "Performing action " + unstoreAction + " on " + incomingDataSetFile; } } diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetHandler.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetHandler.java index 9f1422fb2d2..c338d4e7554 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetHandler.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetHandler.java @@ -173,12 +173,12 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends } @Override - public void rollbackTransaction(DataSetRegistrationService<T> service, + public void didRollbackTransaction(DataSetRegistrationService<T> service, DataSetRegistrationTransaction<T> transaction, DataSetStorageAlgorithmRunner<T> algorithmRunner, Throwable ex) { invokeRollbackTransactionFunction(service, transaction, algorithmRunner, ex); - super.rollbackTransaction(service, transaction, algorithmRunner, ex); + super.didRollbackTransaction(service, transaction, algorithmRunner, ex); } private void invokeRollbackTransactionFunction(DataSetRegistrationService<T> service, diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/api/v1/impl/DataSetRegistrationTransaction.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/api/v1/impl/DataSetRegistrationTransaction.java index 22c1bc18b84..e4506c519ac 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/api/v1/impl/DataSetRegistrationTransaction.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/api/v1/impl/DataSetRegistrationTransaction.java @@ -30,6 +30,7 @@ import ch.systemsx.cisd.common.logging.LogFactory; import ch.systemsx.cisd.etlserver.registrator.DataSetRegistrationDetails; import ch.systemsx.cisd.etlserver.registrator.DataSetRegistrationService; import ch.systemsx.cisd.etlserver.registrator.DataSetStorageAlgorithmRunner; +import ch.systemsx.cisd.etlserver.registrator.IDataSetOnErrorActionDecision.ErrorType; import ch.systemsx.cisd.etlserver.registrator.IDataSetRegistrationDetailsFactory; import ch.systemsx.cisd.etlserver.registrator.IEntityOperationService; import ch.systemsx.cisd.etlserver.registrator.api.v1.IDataSet; @@ -335,10 +336,11 @@ public class DataSetRegistrationTransaction<T extends DataSetInformation> implem /** * Delegate method called by the {@link DataSetStorageAlgorithmRunner}. */ - public void rollback(DataSetStorageAlgorithmRunner<T> algorithm, Throwable ex) + public void didRollbackStorageAlgorithmRunner(DataSetStorageAlgorithmRunner<T> algorithm, + Throwable ex, ErrorType errorType) { rollback(); - registrationService.rollbackTransaction(this, algorithm, ex); + registrationService.didRollbackTransaction(this, algorithm, ex, errorType); } /** diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/ConfiguredOnErrorActionDecisionTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/ConfiguredOnErrorActionDecisionTest.java new file mode 100644 index 00000000000..24e14ff7e83 --- /dev/null +++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/ConfiguredOnErrorActionDecisionTest.java @@ -0,0 +1,79 @@ +/* + * Copyright 2011 ETH Zuerich, CISD + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package ch.systemsx.cisd.etlserver.registrator; + +import java.util.Properties; + +import org.testng.AssertJUnit; +import org.testng.annotations.Test; + +import ch.systemsx.cisd.etlserver.IStorageProcessorTransactional.UnstoreDataAction; +import ch.systemsx.cisd.etlserver.registrator.IDataSetOnErrorActionDecision.ErrorType; + +/** + * @author Chandrasekhar Ramakrishnan + */ +public class ConfiguredOnErrorActionDecisionTest extends AssertJUnit +{ + + @Test + public void testCompletePropertiesFile() + { + Properties props = new Properties(); + props.put(ConfiguredOnErrorActionDecision.INVALID_DATA_SET_KEY, "DELETE"); + props.put(ConfiguredOnErrorActionDecision.OPENBIS_REGISTRATION_FAILURE_KEY, "delete"); + props.put(ConfiguredOnErrorActionDecision.REGISTRATION_SCRIPT_ERROR_KEY, "dElete"); + props.put(ConfiguredOnErrorActionDecision.STORAGE_PROCESSOR_ERROR_KEY, "delete"); + props.put(ConfiguredOnErrorActionDecision.VALIDATION_SCRIPT_ERROR_KEY, "delete"); + props.put(ConfiguredOnErrorActionDecision.POST_REGISTRATION_ERROR_KEY, "delete"); + ConfiguredOnErrorActionDecision decision = new ConfiguredOnErrorActionDecision(props); + assertEquals(UnstoreDataAction.DELETE, + decision.computeUndoAction(ErrorType.INVALID_DATA_SET, null)); + assertEquals(UnstoreDataAction.DELETE, + decision.computeUndoAction(ErrorType.OPENBIS_REGISTRATION_FAILURE, null)); + assertEquals(UnstoreDataAction.DELETE, + decision.computeUndoAction(ErrorType.REGISTRATION_SCRIPT_ERROR, null)); + assertEquals(UnstoreDataAction.DELETE, + decision.computeUndoAction(ErrorType.STORAGE_PROCESSOR_ERROR, null)); + assertEquals(UnstoreDataAction.DELETE, + decision.computeUndoAction(ErrorType.VALIDATION_SCRIPT_ERROR, null)); + assertEquals(UnstoreDataAction.DELETE, + decision.computeUndoAction(ErrorType.POST_REGISTRATION_ERROR, null)); + + } + + @Test + public void testIncompletePropertiesFile() + { + Properties props = new Properties(); + props.put(ConfiguredOnErrorActionDecision.OPENBIS_REGISTRATION_FAILURE_KEY, "move_to_error"); + props.put(ConfiguredOnErrorActionDecision.STORAGE_PROCESSOR_ERROR_KEY, "delete"); + props.put(ConfiguredOnErrorActionDecision.VALIDATION_SCRIPT_ERROR_KEY, "some junk"); + ConfiguredOnErrorActionDecision decision = new ConfiguredOnErrorActionDecision(props); + assertEquals(UnstoreDataAction.LEAVE_UNTOUCHED, + decision.computeUndoAction(ErrorType.INVALID_DATA_SET, null)); + assertEquals(UnstoreDataAction.MOVE_TO_ERROR, + decision.computeUndoAction(ErrorType.OPENBIS_REGISTRATION_FAILURE, null)); + assertEquals(UnstoreDataAction.LEAVE_UNTOUCHED, + decision.computeUndoAction(ErrorType.REGISTRATION_SCRIPT_ERROR, null)); + assertEquals(UnstoreDataAction.DELETE, + decision.computeUndoAction(ErrorType.STORAGE_PROCESSOR_ERROR, null)); + assertEquals(UnstoreDataAction.LEAVE_UNTOUCHED, + decision.computeUndoAction(ErrorType.VALIDATION_SCRIPT_ERROR, null)); + + } +} diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRollbackerTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRollbackerTest.java index 044e0b47647..dc6ec7ab4a9 100644 --- a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRollbackerTest.java +++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageRollbackerTest.java @@ -16,10 +16,8 @@ package ch.systemsx.cisd.etlserver.registrator; -import java.io.BufferedReader; import java.io.File; import java.io.IOException; -import java.io.StringReader; import java.util.Properties; import org.apache.log4j.Logger; @@ -29,7 +27,6 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import ch.systemsx.cisd.base.tests.AbstractFileSystemTestCase; -import ch.systemsx.cisd.common.logging.BufferedAppender; import ch.systemsx.cisd.common.logging.LogCategory; import ch.systemsx.cisd.common.logging.LogFactory; import ch.systemsx.cisd.common.mail.IMailClient; @@ -66,8 +63,6 @@ public class DataSetStorageRollbackerTest extends AbstractFileSystemTestCase private File incomingDataSetFile; - private BufferedAppender logAppender; - private TestDataSetRegistrator testRegistrator; @Override @@ -80,13 +75,11 @@ public class DataSetStorageRollbackerTest extends AbstractFileSystemTestCase openBisService = context.mock(IEncapsulatedOpenBISService.class); dataSetValidator = context.mock(IDataSetValidator.class); mailClient = context.mock(IMailClient.class); - logAppender = new BufferedAppender(); setUpHomeDataBaseExpectations(); TopLevelDataSetRegistratorGlobalState topLevelState = createGlobalState(createThreadProperties()); testRegistrator = new TestDataSetRegistrator(topLevelState); - logAppender.resetLogContent(); } @Test @@ -98,20 +91,16 @@ public class DataSetStorageRollbackerTest extends AbstractFileSystemTestCase DataSetStorageRollbacker rollbacker = new DataSetStorageRollbacker(globalState, logger, UnstoreDataAction.DELETE, incomingDataSetFile, null, null); - rollbacker.appendErrorMessage(); assertEquals( "Performing action DELETE on targets/unit-test-wd/ch.systemsx.cisd.etlserver.registrator.DataSetStorageRollbackerTest/data_set", - logAppender.getLogContent()); - - logAppender.resetLogContent(); + rollbacker.getErrorMessageForLog()); rollbacker = new DataSetStorageRollbacker(globalState, logger, UnstoreDataAction.DELETE, incomingDataSetFile, null, null, ErrorType.REGISTRATION_SCRIPT_ERROR); - rollbacker.appendErrorMessage(); assertEquals( "Responding to error [REGISTRATION_SCRIPT_ERROR] by performing action DELETE on targets/unit-test-wd/ch.systemsx.cisd.etlserver.registrator.DataSetStorageRollbackerTest/data_set", - logAppender.getLogContent()); + rollbacker.getErrorMessageForLog()); } private TopLevelDataSetRegistratorGlobalState createGlobalState(Properties threadProperties) @@ -149,14 +138,12 @@ public class DataSetStorageRollbackerTest extends AbstractFileSystemTestCase protected TestDataSetRegistrator(TopLevelDataSetRegistratorGlobalState globalState) { super(globalState); - // TODO Auto-generated constructor stub } @Override protected void handleDataSet(File dataSetFile, DataSetRegistrationService<DataSetInformation> service) throws Throwable { - // TODO Auto-generated method stub } } @@ -175,33 +162,6 @@ public class DataSetStorageRollbackerTest extends AbstractFileSystemTestCase }); } - protected void assertLogContent(BufferedAppender logRecorder, String... expectedLinesOfContent) - { - BufferedReader reader = new BufferedReader(new StringReader(logRecorder.getLogContent())); - StringBuilder builder = new StringBuilder(); - for (String line : expectedLinesOfContent) - { - builder.append(line).append('\n'); - } - String expectedContent = builder.toString(); - builder.setLength(0); - String line; - try - { - while ((line = reader.readLine()) != null) - { - if (line.startsWith("\tat") == false && line.startsWith("\t... ") == false) - { - builder.append(line).append('\n'); - } - } - } catch (IOException ex) - { - // ignored, because we are reading from a string - } - assertEquals(expectedContent, builder.toString()); - } - private File createDirectory(File parentDir, String directoryName) { final File file = new File(parentDir, directoryName); 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 adb95cda4d7..55f4e6776bc 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 @@ -910,12 +910,12 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractFileSystemTest } @Override - public void rollbackTransaction(DataSetRegistrationService<DataSetInformation> service, + public void didRollbackTransaction(DataSetRegistrationService<DataSetInformation> service, DataSetRegistrationTransaction<DataSetInformation> transaction, DataSetStorageAlgorithmRunner<DataSetInformation> algorithmRunner, Throwable throwable) { - super.rollbackTransaction(service, transaction, algorithmRunner, throwable); + super.didRollbackTransaction(service, transaction, algorithmRunner, throwable); didTransactionRollbackHappen = true; if (shouldReThrowRollbackException) -- GitLab