From a5fa4bfdb1a511e423c25ce2f7456c061aa79a97 Mon Sep 17 00:00:00 2001 From: brinn <brinn> Date: Mon, 21 Jan 2013 19:23:58 +0000 Subject: [PATCH] Add parameters remote-connection-timeout and remote-operation-timeout for setting timeouts of SSH remote connections and operations. This is to support slow hosts and networks (in particular Windows hosts using Cygwin binaries) that always timeout with the default settings. SVN: 28144 --- .../cisd/datamover/DatamoverConstants.java | 4 -- .../systemsx/cisd/datamover/Parameters.java | 37 +++++++++++++- .../cisd/datamover/PropertyNames.java | 4 ++ .../filesystem/FileStoreFactory.java | 23 ++++++--- .../filesystem/intf/AbstractFileStore.java | 11 ++-- .../filesystem/store/FileStoreLocal.java | 3 +- .../filesystem/store/FileStoreRemote.java | 51 ++++++++++++++----- .../store/FileStoreRemoteMounted.java | 5 +- 8 files changed, 105 insertions(+), 33 deletions(-) diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/DatamoverConstants.java b/datamover/source/java/ch/systemsx/cisd/datamover/DatamoverConstants.java index 8990978d551..f8b7b32042f 100644 --- a/datamover/source/java/ch/systemsx/cisd/datamover/DatamoverConstants.java +++ b/datamover/source/java/ch/systemsx/cisd/datamover/DatamoverConstants.java @@ -30,8 +30,4 @@ public class DatamoverConstants public static final String RSYNC_PASSWORD_FILE_OUTGOING = "etc/rsync_outgoing.passwd"; public static final int IGNORED_ERROR_COUNT_BEFORE_NOTIFICATION = 3; - - /** A remote connection must not take longer than 20s to be established. */ - public static final long TIMEOUT_REMOTE_CONNECTION_MILLIS = 20 * 1000L; - } diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/Parameters.java b/datamover/source/java/ch/systemsx/cisd/datamover/Parameters.java index c7b237bf58f..a07a845f4c7 100644 --- a/datamover/source/java/ch/systemsx/cisd/datamover/Parameters.java +++ b/datamover/source/java/ch/systemsx/cisd/datamover/Parameters.java @@ -50,6 +50,7 @@ import ch.systemsx.cisd.common.properties.PropertyUtils; import ch.systemsx.cisd.datamover.filesystem.FileStoreFactory; import ch.systemsx.cisd.datamover.filesystem.intf.IFileStore; import ch.systemsx.cisd.datamover.filesystem.intf.IFileSysOperationsFactory; +import ch.systemsx.cisd.datamover.filesystem.store.FileStoreRemote; import ch.systemsx.cisd.datamover.intf.IFileSysParameters; import ch.systemsx.cisd.datamover.intf.ITimingParameters; @@ -82,6 +83,16 @@ public final class Parameters implements ITimingParameters, IFileSysParameters + "[default: " + DEFAULT_DATA_COMPLETED_SCRIPT_TIMEOUT + "]", handler = MillisecondConversionOptionHandler.class) private long dataCompletedScriptTimeout = DEFAULT_DATA_COMPLETED_SCRIPT_TIMEOUT; + @Option(longName = PropertyNames.REMOTE_CONNECTION_TIMEOUT, usage = "Timeout (in seconds) for a remote connection to be established " + + "[default: " + FileStoreRemote.DEFAULT_REMOTE_OPERATION_TIMEOUT_MILLIS / 1000L + "]", handler = MillisecondConversionOptionHandler.class) + private long remoteConnectionTimeout = + FileStoreRemote.DEFAULT_REMOTE_OPERATION_TIMEOUT_MILLIS / 1000L; + + @Option(longName = PropertyNames.REMOTE_OPERATION_TIMEOUT, usage = "Timeout (in seconds) for a remote operations to complete " + + "[default: " + FileStoreRemote.DEFAULT_REMOTE_OPERATION_TIMEOUT_MILLIS / 1000L + "]", handler = MillisecondConversionOptionHandler.class) + private long remoteOperationTimeout = + FileStoreRemote.DEFAULT_REMOTE_OPERATION_TIMEOUT_MILLIS / 1000L; + /** * The name of the <code>rsync</code> executable to use for copy operations. */ @@ -471,6 +482,12 @@ public final class Parameters implements ITimingParameters, IFileSysParameters dataCompletedScriptTimeout = toMillis(PropertyUtils.getPosLong(serviceProperties, PropertyNames.DATA_COMPLETED_SCRIPT_TIMEOUT, dataCompletedScriptTimeout)); + remoteConnectionTimeout = + toMillis(PropertyUtils.getPosLong(serviceProperties, + PropertyNames.REMOTE_CONNECTION_TIMEOUT, remoteConnectionTimeout)); + remoteOperationTimeout = + toMillis(PropertyUtils.getPosLong(serviceProperties, + PropertyNames.REMOTE_OPERATION_TIMEOUT, remoteOperationTimeout)); rsyncExecutable = PropertyUtils.getProperty(serviceProperties, PropertyNames.RSYNC_EXECUTABLE, rsyncExecutable); @@ -617,6 +634,16 @@ public final class Parameters implements ITimingParameters, IFileSysParameters return dataCompletedScriptTimeout; } + public final long getRemoteConnectionTimeout() + { + return remoteConnectionTimeout; + } + + public final long getRemoteOperationTimeout() + { + return remoteOperationTimeout; + } + /** * @return The name of the <code>rsync</code> executable to use for copy operations. */ @@ -777,7 +804,8 @@ public final class Parameters implements ITimingParameters, IFileSysParameters FileStoreFactory.createStore(incomingTarget, INCOMING_KIND_DESC, treatIncomingAsRemote, factory, skipAccessibilityTestOnIncoming, incomingHostFindExecutableOrNull, - incomingHostLastchangedExecutableOrNull, checkIntervalMillis); + incomingHostLastchangedExecutableOrNull, checkIntervalMillis, + remoteConnectionTimeout, remoteOperationTimeout); } return incomingStore; } @@ -834,7 +862,8 @@ public final class Parameters implements ITimingParameters, IFileSysParameters outgoingStore = FileStoreFactory.createStore(outgoingTarget, OUTGOING_KIND_DESC, true, factory, skipAccessibilityTestOnOutgoing, outgoingHostFindExecutableOrNull, - outgoingHostLastchangedExecutableOrNull, checkIntervalMillis); + outgoingHostLastchangedExecutableOrNull, checkIntervalMillis, + remoteConnectionTimeout, remoteOperationTimeout); } return outgoingStore; } @@ -940,6 +969,10 @@ public final class Parameters implements ITimingParameters, IFileSysParameters DurationFormatUtils.formatDuration(getIntervalToWaitAfterFailure(), "s"))); operationLog.info(String.format("Maximum number of retries: %d.", getMaximalNumberOfRetries())); + operationLog.info(String.format("Timeout for remote connections: %s s.", + DurationFormatUtils.formatDuration(getRemoteConnectionTimeout(), "s"))); + operationLog.info(String.format("Timeout for remote operations: %s s.", + DurationFormatUtils.formatDuration(getRemoteOperationTimeout(), "s"))); if (tryGetCleansingRegex() != null) { operationLog.info(String.format( diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/PropertyNames.java b/datamover/source/java/ch/systemsx/cisd/datamover/PropertyNames.java index 0026a1d02cc..019fb81c6b8 100644 --- a/datamover/source/java/ch/systemsx/cisd/datamover/PropertyNames.java +++ b/datamover/source/java/ch/systemsx/cisd/datamover/PropertyNames.java @@ -37,6 +37,10 @@ public final class PropertyNames static final String DATA_COMPLETED_SCRIPT_TIMEOUT = "data-completed-script-timeout"; + static final String REMOTE_CONNECTION_TIMEOUT = "remote-connection-timeout"; + + static final String REMOTE_OPERATION_TIMEOUT = "remote-operation-timeout"; + static final String CLEANSING_REGEX = "cleansing-regex"; /** The local directory where we create additional copy of the incoming data. */ diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/FileStoreFactory.java b/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/FileStoreFactory.java index 0a134253ead..4d6afb4e5d1 100644 --- a/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/FileStoreFactory.java +++ b/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/FileStoreFactory.java @@ -50,10 +50,12 @@ public final class FileStoreFactory private static final IFileStore createRemoteHost(final HostAwareFileWithHighwaterMark path, final String kind, final IFileSysOperationsFactory factory, final boolean skipAccessibilityTest, final String findExecutableOrNull, - final String lastchangedExecutableOrNull) + final String lastchangedExecutableOrNull, final long remoteConnectionTimeoutMillis, + final long remoteOperationTimeoutMillis) { return new FileStoreRemote(path, kind, factory, skipAccessibilityTest, - findExecutableOrNull, lastchangedExecutableOrNull); + findExecutableOrNull, lastchangedExecutableOrNull, remoteConnectionTimeoutMillis, + remoteOperationTimeoutMillis); } /** @@ -87,7 +89,9 @@ public final class FileStoreFactory final IFileSysOperationsFactory factory) { return createRemoteHost(new HostAwareFileWithHighwaterMark(host, path, rsyncModuleOrNull), - kind, factory, false, null, null); + kind, factory, false, null, null, + FileStoreRemote.DEFAULT_REMOTE_CONNECTION_TIMEOUT_MILLIS, + FileStoreRemote.DEFAULT_REMOTE_OPERATION_TIMEOUT_MILLIS); } /** @@ -99,19 +103,22 @@ public final class FileStoreFactory public final static IFileStore createStore(final HostAwareFileWithHighwaterMark path, final String kind, final boolean isRemote, final IFileSysOperationsFactory factory, final boolean skipAccessibilityTest, final String findExecutableOrNull, - final String lastchangedExecutableOrNull, final long checkIntervalMillis) + final String lastchangedExecutableOrNull, final long checkIntervalMillis, + final long remoteConnectionTimeoutMillis, final long remoteOperationTimeoutMillis) { if (path.tryGetHost() != null) { if (operationLog.isDebugEnabled()) { operationLog.debug(String.format( - "Create %s store for remote host %s, path %s, timeout: %f s.", kind, - path.tryGetHost(), path.getPath(), - FileStoreRemote.LONG_SSH_TIMEOUT_MILLIS / 1000.0)); + "Create %s store for remote host %s, path %s, connection timeout: %d s, " + + "operation timeout: %d s.", kind, path.tryGetHost(), + path.getPath(), remoteConnectionTimeoutMillis / 1000, + remoteOperationTimeoutMillis / 1000)); } return createRemoteHost(path, kind, factory, skipAccessibilityTest, - findExecutableOrNull, lastchangedExecutableOrNull); + findExecutableOrNull, lastchangedExecutableOrNull, + remoteConnectionTimeoutMillis, remoteOperationTimeoutMillis); } else { final long timoutMillis = checkIntervalMillis * 3; diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/intf/AbstractFileStore.java b/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/intf/AbstractFileStore.java index 217bcb4a6a4..26c2d10de41 100644 --- a/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/intf/AbstractFileStore.java +++ b/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/intf/AbstractFileStore.java @@ -42,6 +42,8 @@ import ch.systemsx.cisd.datamover.DatamoverConstants; */ public abstract class AbstractFileStore implements IFileStore { + public static final long DEFAULT_REMOTE_CONNECTION_TIMEOUT_MILLIS = 20 * 1000L; + private final HostAwareFileWithHighwaterMark hostAwareFileWithHighwaterMark; private final String kind; @@ -50,12 +52,15 @@ public abstract class AbstractFileStore implements IFileStore protected final boolean skipAccessibilityTest; + protected final long remoteConnectionTimeoutMillis; + protected AbstractFileStore( final HostAwareFileWithHighwaterMark hostAwareFileWithHighwaterMark, final String kind, - final IFileSysOperationsFactory factory, final boolean skipAccessibilityTest) + final IFileSysOperationsFactory factory, final boolean skipAccessibilityTest, final long remoteConnectionTimeoutMillis) { assert hostAwareFileWithHighwaterMark != null; assert kind != null; + this.remoteConnectionTimeoutMillis = remoteConnectionTimeoutMillis; this.hostAwareFileWithHighwaterMark = hostAwareFileWithHighwaterMark; this.kind = kind; this.factory = factory; @@ -163,7 +168,7 @@ public abstract class AbstractFileStore implements IFileStore FileUtilities.checkPathCopier(copier, srcHostOrNull, executableOrNull, tryGetRsyncModuleName(), DatamoverConstants.RSYNC_PASSWORD_FILE_INCOMING, - DatamoverConstants.TIMEOUT_REMOTE_CONNECTION_MILLIS); + remoteConnectionTimeoutMillis); } if (destHostOrNull != null) { @@ -171,7 +176,7 @@ public abstract class AbstractFileStore implements IFileStore FileUtilities.checkPathCopier(copier, destHostOrNull, executableOrNull, destinationStore.tryGetRsyncModuleName(), DatamoverConstants.RSYNC_PASSWORD_FILE_OUTGOING, - DatamoverConstants.TIMEOUT_REMOTE_CONNECTION_MILLIS); + remoteConnectionTimeoutMillis); } } diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreLocal.java b/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreLocal.java index f190149a5e7..faf16a035c7 100644 --- a/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreLocal.java +++ b/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreLocal.java @@ -67,7 +67,8 @@ public class FileStoreLocal extends AbstractFileStore implements IExtendedFileSt final String description, final IFileSysOperationsFactory factory, final boolean skipAccessibilityTest) { - super(hostAwareFileWithHighwaterMark, description, factory, skipAccessibilityTest); + super(hostAwareFileWithHighwaterMark, description, factory, skipAccessibilityTest, + DEFAULT_REMOTE_CONNECTION_TIMEOUT_MILLIS); this.remover = factory.getRemover(); this.mover = factory.getMover(); this.highwaterMarkWatcher = createHighwaterMarkWatcher(hostAwareFileWithHighwaterMark); diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreRemote.java b/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreRemote.java index ce3f2bd141b..660b2cdaf6c 100644 --- a/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreRemote.java +++ b/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreRemote.java @@ -57,9 +57,9 @@ public class FileStoreRemote extends AbstractFileStore private static final String NO_SUCH_FILE_OR_DIRECTORY_MSG = "No such file or directory"; - public static final long QUICK_SSH_TIMEOUT_MILLIS = 15 * 1000; + public static final long DEFAULT_REMOTE_OPERATION_TIMEOUT_MILLIS = 100 * 1000; - public static final long LONG_SSH_TIMEOUT_MILLIS = 120 * 1000; + private final long remoteOperationAndConnectionTimeoutMillis; // -- bash commands ------------- @@ -126,11 +126,13 @@ public class FileStoreRemote extends AbstractFileStore public FileStoreRemote(final HostAwareFileWithHighwaterMark fileWithHighwaterMark, final String kind, final IFileSysOperationsFactory factory, final boolean skipAccessibilityTest, final String remoteFindExecutableOrNull, - final String remoteLastchangedExecutableOrNull) + final String remoteLastchangedExecutableOrNull, + final long remoteConnectionTimeoutMillis, final long remoteOperationTimeoutMillis) { this(fileWithHighwaterMark, kind, SshCommandExecutor .createSshCommandBuilder(findSSHOrDie(factory)), factory, skipAccessibilityTest, - remoteFindExecutableOrNull, remoteLastchangedExecutableOrNull); + remoteFindExecutableOrNull, remoteLastchangedExecutableOrNull, + remoteConnectionTimeoutMillis, remoteOperationTimeoutMillis); } @Private @@ -139,13 +141,30 @@ public class FileStoreRemote extends AbstractFileStore final IFileSysOperationsFactory factory, final boolean skipAccessibilityTest, final String remoteFindExecutableOrNull, final String remoteLastchangedExecutableOrNull) { - super(hostAwareFileWithHighwaterMark, kind, factory, skipAccessibilityTest); + this(hostAwareFileWithHighwaterMark, kind, sshCommandBuilder, factory, + skipAccessibilityTest, remoteFindExecutableOrNull, + remoteLastchangedExecutableOrNull, DEFAULT_REMOTE_CONNECTION_TIMEOUT_MILLIS, + DEFAULT_REMOTE_OPERATION_TIMEOUT_MILLIS); + } + + @Private + FileStoreRemote(final HostAwareFileWithHighwaterMark hostAwareFileWithHighwaterMark, + final String kind, final ISshCommandBuilder sshCommandBuilder, + final IFileSysOperationsFactory factory, final boolean skipAccessibilityTest, + final String remoteFindExecutableOrNull, + final String remoteLastchangedExecutableOrNull, + final long remoteConnectionTimeoutMillis, final long remoteOperationTimeoutMillis) + { + super(hostAwareFileWithHighwaterMark, kind, factory, skipAccessibilityTest, + remoteConnectionTimeoutMillis); assert hostAwareFileWithHighwaterMark.tryGetHost() != null : "Unspecified host"; this.sshCommandExecutor = new SshCommandExecutor(sshCommandBuilder, hostAwareFileWithHighwaterMark.tryGetHost()); this.highwaterMarkWatcher = createHighwaterMarkWatcher(hostAwareFileWithHighwaterMark, sshCommandBuilder); + this.remoteOperationAndConnectionTimeoutMillis = + remoteOperationTimeoutMillis + remoteConnectionTimeoutMillis; setAndLogLastchangedExecutable(remoteLastchangedExecutableOrNull); setAndLogFindExecutable(remoteFindExecutableOrNull); if (remoteFindExecutableOrNull != null) @@ -202,7 +221,7 @@ public class FileStoreRemote extends AbstractFileStore final String pathString = toUnixPathString(item); final String cmd = mkDeleteFileCommand(pathString); final ProcessResult result = - sshCommandExecutor.executeCommandRemotely(cmd, QUICK_SSH_TIMEOUT_MILLIS); + sshCommandExecutor.executeCommandRemotely(cmd, remoteConnectionTimeoutMillis); final String errMsg = getErrorMessageOrNull(result); if (errMsg == null) { @@ -217,7 +236,7 @@ public class FileStoreRemote extends AbstractFileStore public final BooleanStatus exists(final StoreItem item) { final String pathString = toUnixPathString(item); - return sshCommandExecutor.exists(pathString, QUICK_SSH_TIMEOUT_MILLIS); + return sshCommandExecutor.exists(pathString, remoteConnectionTimeoutMillis); } @Override @@ -249,7 +268,8 @@ public class FileStoreRemote extends AbstractFileStore mkLastchangedCommand(itemPath, stopWhenFindYoungerMillis, isRelative, remoteLastchangedExecutableOrNull); final ProcessResult result = - sshCommandExecutor.executeCommandRemotely(cmd, LONG_SSH_TIMEOUT_MILLIS); + sshCommandExecutor.executeCommandRemotely(cmd, + remoteOperationAndConnectionTimeoutMillis); final String errMsg = getErrorMessageOrNullForLastchanged(result); if (errMsg == null) { @@ -269,7 +289,8 @@ public class FileStoreRemote extends AbstractFileStore final String findExec = getRemoteFindExecutableOrDie(); final String cmd = mkFindYoungestModificationTimestampSecCommand(itemPath, findExec); final ProcessResult result = - sshCommandExecutor.executeCommandRemotely(cmd, LONG_SSH_TIMEOUT_MILLIS); + sshCommandExecutor.executeCommandRemotely(cmd, + remoteOperationAndConnectionTimeoutMillis); final String errMsg = getErrorMessageOrNullForFind(result); if (errMsg == null) { @@ -398,7 +419,8 @@ public class FileStoreRemote extends AbstractFileStore { final String cmd = mkCheckCommandExistsCommand(findExec); final ProcessResult result = - sshCommandExecutor.executeCommandRemotely(cmd, QUICK_SSH_TIMEOUT_MILLIS, false); + sshCommandExecutor + .executeCommandRemotely(cmd, remoteConnectionTimeoutMillis, false); if (machineLog.isDebugEnabled()) { result.log(); @@ -408,7 +430,8 @@ public class FileStoreRemote extends AbstractFileStore final String findExecutable = result.getOutput().get(0); final String verCmd = getVersionCommand(findExec); final ProcessResult verResult = - sshCommandExecutor.executeCommandRemotely(verCmd, QUICK_SSH_TIMEOUT_MILLIS, + sshCommandExecutor.executeCommandRemotely(verCmd, + remoteConnectionTimeoutMillis, false); if (machineLog.isDebugEnabled()) { @@ -456,7 +479,8 @@ public class FileStoreRemote extends AbstractFileStore { final String cmd = mkCheckCommandExistsCommand(exec); final ProcessResult result = - sshCommandExecutor.executeCommandRemotely(cmd, QUICK_SSH_TIMEOUT_MILLIS, false); + sshCommandExecutor.executeCommandRemotely(cmd, remoteConnectionTimeoutMillis + , false); if (machineLog.isDebugEnabled()) { result.log(); @@ -508,7 +532,8 @@ public class FileStoreRemote extends AbstractFileStore { final String simpleCmd = mkListByOldestModifiedCommand(toUnixPathString(null)); final ProcessResult result = - sshCommandExecutor.executeCommandRemotely(simpleCmd, LONG_SSH_TIMEOUT_MILLIS); + sshCommandExecutor.executeCommandRemotely(simpleCmd, + remoteOperationAndConnectionTimeoutMillis); if (result.isOK()) { return asStoreItems(result.getOutput()); diff --git a/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreRemoteMounted.java b/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreRemoteMounted.java index 2aac3dfc881..eb0056324fa 100644 --- a/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreRemoteMounted.java +++ b/datamover/source/java/ch/systemsx/cisd/datamover/filesystem/store/FileStoreRemoteMounted.java @@ -57,7 +57,8 @@ public final class FileStoreRemoteMounted extends AbstractFileStore final String description, final IFileSysOperationsFactory factory, final boolean skipAccessibilityTest, final long lastChangedTimeoutMillis) { - super(file, description, factory, skipAccessibilityTest); + super(file, description, factory, skipAccessibilityTest, + DEFAULT_REMOTE_CONNECTION_TIMEOUT_MILLIS); this.localImpl = new FileStoreLocal(file, description, factory, skipAccessibilityTest); this.localImplMonitored = MonitoringProxy.create(IFileStore.class, localImpl) @@ -188,7 +189,7 @@ public final class FileStoreRemoteMounted extends AbstractFileStore { log(level, message, null); } - + @Override public void log(LogLevel level, String message, Throwable throwableOrNull) { -- GitLab