From 62fa46c63d138932b5a393e6bf9e9b0f72f2ffac Mon Sep 17 00:00:00 2001
From: cramakri <cramakri>
Date: Tue, 4 Oct 2011 21:05:36 +0000
Subject: [PATCH] LMS-2570 Changes to way storage processors are executed and
rolled back to make rollbacks more robust in the face of unreliable file
systems
SVN: 23185
---
.../etlserver/DefaultStorageProcessor.java | 12 +-
.../registrator/DataSetStorageAlgorithm.java | 25 +---
.../DataSetStorageAlgorithmRunner.java | 7 +-
.../etlserver/registrator/IRollbackStack.java | 9 --
...opLevelDataSetRegistratorRollbackTest.java | 124 +++++++-----------
5 files changed, 66 insertions(+), 111 deletions(-)
diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/DefaultStorageProcessor.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/DefaultStorageProcessor.java
index 42217f41c43..29f20fc6535 100644
--- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/DefaultStorageProcessor.java
+++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/DefaultStorageProcessor.java
@@ -127,6 +127,11 @@ public class DefaultStorageProcessor extends AbstractStorageProcessor
@Override
protected UnstoreDataAction executeRollback(Throwable ex)
{
+ // This might happen when the transaction is serialized and de-serialized.
+ if (null == storedDataDirectory)
+ {
+ storedDataDirectory = rootDirectory;
+ }
checkParameters(incomingDataSetDirectory, storedDataDirectory);
File targetFile =
new File(getOriginalDirectory(storedDataDirectory),
@@ -138,9 +143,10 @@ public class DefaultStorageProcessor extends AbstractStorageProcessor
FileUtils.deleteDirectory(storedDataDirectory);
} catch (IOException ex1)
{
- String message = String.format("Failed to remove stored directory '%s'. " +
- "In the future the creation of a data set with the same code will fail. " +
- "To fix the problem remove the directory manually.");
+ String message =
+ String.format("Failed to remove stored directory '%s'. "
+ + "In the future the creation of a data set with the same code will fail. "
+ + "To fix the problem remove the directory manually.");
operationLog.warn(message);
}
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 f140be84508..34162636978 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
@@ -161,9 +161,10 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation>
}
/**
- * Ask the storage processor to rollback. Used by clients of the algorithm.
+ * Transition to the rolledback state, but don't actually do anything. The rollback logic will
+ * be carried out by the rollback stack.
*/
- public void rollbackStorageProcessor(Throwable throwable)
+ public void transitionToRolledbackState(Throwable throwable)
{
// Rollback may be called on in the stored state or in the prepared state.
if (state instanceof PreparedState)
@@ -182,12 +183,12 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation>
}
StoredState<T> storedState = (StoredState<T>) state;
- UnstoreDataAction action = storedState.rollbackStorageProcessor(throwable);
+ storedState.cleanUp();
- state = new RolledbackState<T>(storedState, action, throwable);
+ state = new RolledbackState<T>(storedState, UnstoreDataAction.LEAVE_UNTOUCHED, throwable);
}
- public void executeUndoStoreAction()
+ public void transitionToUndoneState()
{
// Rollback may be called on in the stored state or in the prepared state. In the prepared
// state, there is nothing to do.
@@ -199,10 +200,6 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation>
RolledbackState<T> rolledbackState = (RolledbackState<T>) state;
- // Do not undo the individual data sets -- the entire transaction needs to be undone as a
- // group
- // rolledbackState.executeUndoAction();
-
state = new UndoneState<T>(rolledbackState);
}
@@ -445,16 +442,6 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation>
this.markerFile = oldState.markerFile;
}
- /**
- * Ask the storage processor to rollback. Used by clients of the algorithm.
- */
- public UnstoreDataAction rollbackStorageProcessor(final Throwable throwable)
- {
- UnstoreDataAction action = transaction.rollback(throwable);
- cleanUp();
- return action;
- }
-
/**
* Ask the storage processor to commit. Used by clients of the algorithm.
*/
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 aefecd97811..0b874d51e50 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
@@ -254,7 +254,6 @@ public class DataSetStorageAlgorithmRunner<T extends DataSetInformation>
}
}
- @SuppressWarnings("deprecation")
private void rollbackStorageProcessors(Throwable ex)
{
operationLog.error(
@@ -272,10 +271,8 @@ public class DataSetStorageAlgorithmRunner<T extends DataSetInformation>
for (int i = dataSetStorageAlgorithms.size() - 1; i >= 0; --i)
{
DataSetStorageAlgorithm<T> storageAlgorithm = dataSetStorageAlgorithms.get(i);
- storageAlgorithm.rollbackStorageProcessor(ex);
- storageAlgorithm.executeUndoStoreAction();
- // remove the serialized transaction
- rollbackStack.pop();
+ storageAlgorithm.transitionToRolledbackState(ex);
+ storageAlgorithm.transitionToUndoneState();
}
}
diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IRollbackStack.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IRollbackStack.java
index 37580401570..712de5d26ef 100644
--- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IRollbackStack.java
+++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IRollbackStack.java
@@ -29,13 +29,4 @@ public interface IRollbackStack
* Push the command onto the stack and execute it.
*/
void pushAndExecuteCommand(ITransactionalCommand cmd);
-
- /**
- * Pop the value from the top of the stack *without* rollbacking.
- * <p>
- * This method should not be called
- */
- @Deprecated
- ITransactionalCommand pop();
-
}
\ No newline at end of file
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorRollbackTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorRollbackTest.java
index 90ddfa01a45..9fb200a765c 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorRollbackTest.java
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorRollbackTest.java
@@ -20,14 +20,14 @@ import static ch.systemsx.cisd.common.Constants.IS_FINISHED_PREFIX;
import java.io.File;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.Arrays;
-import java.util.List;
import java.util.Properties;
import org.apache.commons.io.FileUtils;
import org.hamcrest.core.IsAnything;
import org.jmock.Expectations;
+import org.jmock.api.Invocation;
+import org.jmock.lib.action.CustomAction;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
@@ -35,6 +35,8 @@ import ch.systemsx.cisd.base.exceptions.IOExceptionUnchecked;
import ch.systemsx.cisd.common.filesystem.FileUtilities;
import ch.systemsx.cisd.common.mail.IMailClient;
import ch.systemsx.cisd.common.utilities.ExtendedProperties;
+import ch.systemsx.cisd.etlserver.AbstractStorageProcessorTransaction;
+import ch.systemsx.cisd.etlserver.DefaultStorageProcessor;
import ch.systemsx.cisd.etlserver.IStorageProcessorTransactional;
import ch.systemsx.cisd.etlserver.ITypeExtractor;
import ch.systemsx.cisd.etlserver.registrator.api.v1.impl.DataSetRegistrationTransaction;
@@ -44,7 +46,6 @@ import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.SearchCriteria.MatchCl
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetType;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Experiment;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.builders.ExperimentBuilder;
-import ch.systemsx.cisd.openbis.generic.shared.dto.StorageFormat;
import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifierFactory;
/**
@@ -78,7 +79,7 @@ public class JythonTopLevelDataSetRegistratorRollbackTest extends AbstractJython
Properties properties =
createThreadPropertiesRelativeToScriptsFolder("simple-transaction.py");
properties.setProperty(IStorageProcessorTransactional.STORAGE_PROCESSOR_KEY,
- MockStorageProcessor.class.getName());
+ DefaultStorageProcessor.class.getName());
createHandler(properties, false, false);
createData();
@@ -99,7 +100,19 @@ public class JythonTopLevelDataSetRegistratorRollbackTest extends AbstractJython
one(openBisService)
.performEntityOperations(
with(new IsAnything<ch.systemsx.cisd.openbis.generic.shared.dto.AtomicEntityOperationDetails>()));
- will(throwException(new AssertionError("Fail")));
+
+ CustomAction makeFileSystemUnavailable = new CustomAction("foo")
+ {
+
+ @Override
+ public Object invoke(Invocation invocation) throws Throwable
+ {
+ makeFileSystemUnavailable(workingDirectory);
+ return null;
+ }
+ };
+ will(doAll(makeFileSystemUnavailable,
+ throwException(new AssertionError("Fail"))));
}
});
@@ -113,8 +126,6 @@ public class JythonTopLevelDataSetRegistratorRollbackTest extends AbstractJython
makeFileSystemAvailable(workingDirectory);
DataSetRegistrationTransaction.rollbackDeadTransactions(workingDirectory);
}
- assertEquals(1, MockStorageProcessor.instance.incomingDirs.size());
- assertEquals(0, MockStorageProcessor.instance.calledCommitCount);
assertEquals("[]", Arrays.asList(stagingDirectory.list()).toString());
assertEquals(
"hello world1",
@@ -125,12 +136,6 @@ public class JythonTopLevelDataSetRegistratorRollbackTest extends AbstractJython
FileUtilities.loadToString(
new File(workingDirectory, "data_set/sub_data_set_2/read2.me")).trim());
- TestingDataSetHandler theHandler = (TestingDataSetHandler) handler;
- assertFalse(theHandler.didRollbackDataSetRegistrationFunctionRun);
- assertFalse(theHandler.didRollbackServiceFunctionRun);
- assertTrue(theHandler.didTransactionRollbackHappen);
- assertTrue(theHandler.didRollbackTransactionFunctionRunHappen);
-
context.assertIsSatisfied();
}
@@ -167,104 +172,73 @@ public class JythonTopLevelDataSetRegistratorRollbackTest extends AbstractJython
{
private static final long serialVersionUID = 1L;
- static MockStorageProcessor instance;
-
- int calledGetStoreRootDirectoryCount = 0;
-
- int calledCommitCount = 0;
-
- File storeRootDirectory;
-
- String dataSetInfoString;
-
- protected List<File> incomingDirs = new ArrayList<File>();
-
- protected List<File> rootDirs = new ArrayList<File>();
-
public MockStorageProcessor(ExtendedProperties props)
{
super(props);
instance = this;
}
- public File getStoreRootDirectory()
- {
- calledGetStoreRootDirectoryCount++;
- return storeRootDirectory;
- }
-
- public void setStoreRootDirectory(File storeRootDirectory)
- {
- this.storeRootDirectory = storeRootDirectory;
- }
-
- public StorageFormat getStorageFormat()
- {
- return StorageFormat.PROPRIETARY;
- }
-
- public UnstoreDataAction getDefaultUnstoreDataAction(Throwable exception)
- {
- return UnstoreDataAction.LEAVE_UNTOUCHED;
- }
-
+ @Override
public IStorageProcessorTransaction createTransaction(
- StorageProcessorTransactionParameters parameters)
+ final StorageProcessorTransactionParameters parameters)
{
- final File rootDir = parameters.getRootDir();
- dataSetInfoString = parameters.getDataSetInformation().toString();
- return new IStorageProcessorTransaction()
+ return new AbstractStorageProcessorTransaction(parameters)
{
private static final long serialVersionUID = 1L;
- private File storedFolder;
-
- public void storeData(ITypeExtractor typeExtractor, IMailClient mailClient,
- File incomingDataSetFile)
+ @Override
+ public File tryGetProprietaryData()
{
+ return null;
+ }
- incomingDirs.add(incomingDataSetFile);
- rootDirs.add(rootDir);
-
+ @Override
+ protected File executeStoreData(ITypeExtractor typeExtractor,
+ IMailClient mailClient)
+ {
try
{
- if (incomingDataSetFile.isDirectory())
+ if (incomingDataSetDirectory.isDirectory())
{
- FileUtils.moveDirectoryToDirectory(incomingDataSetFile, new File(
- rootDir, "original"), true);
+ FileUtils.moveDirectoryToDirectory(incomingDataSetDirectory,
+ new File(rootDirectory, "original"), true);
} else
{
- FileUtils.moveFileToDirectory(incomingDataSetFile, new File(
- rootDir, "original"), false);
+ FileUtils.moveFileToDirectory(incomingDataSetDirectory, new File(
+ rootDirectory, "original"), false);
}
makeFileSystemUnavailable(getStoreRootDirectory());
} catch (IOException ex)
{
throw new IOExceptionUnchecked(ex);
}
- storedFolder = rootDir;
+ return rootDirectory;
}
- public UnstoreDataAction rollback(Throwable exception)
+ public UnstoreDataAction executeRollback(Throwable exception)
{
- return null;
+ rollback(incomingDataSetDirectory, rootDirectory);
+ return UnstoreDataAction.LEAVE_UNTOUCHED;
}
- public File getStoredDataDirectory()
+ private void rollback(File incomingDataSetFile, File aRootDir)
{
- return storedFolder;
+ try
+ {
+ FileUtils.moveDirectoryToDirectory(new File(aRootDir, "original"),
+ incomingDataSetFile, true);
+ } catch (IOException ex)
+ {
+ throw new IOExceptionUnchecked(ex);
+ }
}
- public void commit()
+ @Override
+ protected void executeCommit()
{
calledCommitCount++;
}
-
- public File tryGetProprietaryData()
- {
- return null;
- }
};
}
}
--
GitLab