Skip to content

Commit

Permalink
srm: add short request lifetime work-around
Browse files Browse the repository at this point in the history 
Motivation:

Some clients make requests with ridiculously short
desiredTotalRequestTime values and users are surprised when these
requests fail and open support tickets.

Modification:

Add a sanity check, based on an admin's estimate of the minimum bandwidth
that file transfers will likely suffer.  If the requested lifetime is
shorter than the estimated transfer under worse conditions, then the
lifetime is extended up to the configured maximum value.

Result:

Request from broken clients are more likely to succeed.

Target: master
Patch: https://rb.dcache.org/r/9123
Acked-by: Gerd Behrmann
Request: 2.15
Request: 2.14
Request: 2.13
Request: 2.12
Request: 2.11
Request: 2.10
  • Loading branch information
@paulmillar
paulmillar committed Mar 21, 2016
1 parent a8d4ca0 commit b13fa36
Show file tree
Hide file tree
Showing 11 changed files with 215 additions and 98 deletions.
1 change: 1 addition & 0 deletions modules/dcache-srm/src/main/resources/diskCacheV111/srm/srm.xml
View file
Expand Up @@ -469,6 +469,7 @@
<property name="prefix" value="srm.ping-extra-info"/>
</bean>
</property>
<property name="maximumClientAssumedBandwidth" value="${srm.request.maximum-client-assumed-bandwidth}"/>
</bean>

<bean id="scheduling-strategy-provider" class="org.dcache.srm.scheduler.SchedulingStrategyFactoryBean">
Expand Down
23 changes: 2 additions & 21 deletions modules/srm-server/src/main/java/org/dcache/srm/handler/SrmBringOnline.java
View file
Expand Up @@ -18,6 +18,7 @@
import org.dcache.srm.scheduler.IllegalStateTransition;
import org.dcache.srm.util.Configuration;
import org.dcache.srm.util.JDC;
import org.dcache.srm.util.Lifetimes;
import org.dcache.srm.util.Tools;
import org.dcache.srm.v2_2.ArrayOfTExtraInfo;
import org.dcache.srm.v2_2.SrmBringOnlineRequest;
Expand Down Expand Up @@ -83,7 +84,7 @@ private SrmBringOnlineResponse srmBringOnline()
String clientHost = getClientNetwork(request).or(this.clientHost);
TGetFileRequest[] fileRequests = getFileRequests(request);
URI[] surls = getSurls(fileRequests);
long requestTime = getRequestTime(request, configuration.getBringOnlineLifetime());
long requestTime = Lifetimes.calculateLifetime(request.getDesiredTotalRequestTime(), configuration.getBringOnlineLifetime());
long desiredLifetimeInSeconds = getDesiredLifetime(request, requestTime);

if (protocols != null && protocols.length > 0) {
Expand Down Expand Up @@ -130,26 +131,6 @@ private static long getDesiredLifetime(SrmBringOnlineRequest request, long reque
return (long) request.getDesiredLifeTime();
}

private static long getRequestTime(SrmBringOnlineRequest request, long max) throws SRMInvalidRequestException
{
long requestTime = 0;
if (request.getDesiredTotalRequestTime() != null) {
long time = request.getDesiredTotalRequestTime();
/* [ SRM 2.2, 5.3.2 ]
*
* o) If input parameter desiredTotalRequestTime is 0 (zero), each file request must
* be tried at least once. Negative value must be invalid.
*/
if (time < 0) {
throw new SRMInvalidRequestException("destiredTotalRequestTime must not be negative.");
}
requestTime = time;
}
/* FIXME: The interpretation of 0 does not match the SRM spec.
*/
return (requestTime > 0) ? TimeUnit.SECONDS.toMillis(requestTime) : max;
}

private static String getExtraInfo(SrmBringOnlineRequest request, String key)
{
ArrayOfTExtraInfo storageSystemInfo = request.getStorageSystemInfo();
Expand Down
27 changes: 2 additions & 25 deletions modules/srm-server/src/main/java/org/dcache/srm/handler/SrmCopy.java
View file
Expand Up @@ -18,6 +18,7 @@
import org.dcache.srm.scheduler.IllegalStateTransition;
import org.dcache.srm.util.Configuration;
import org.dcache.srm.util.JDC;
import org.dcache.srm.util.Lifetimes;
import org.dcache.srm.v2_2.ArrayOfTExtraInfo;
import org.dcache.srm.v2_2.SrmCopyRequest;
import org.dcache.srm.v2_2.SrmCopyResponse;
Expand Down Expand Up @@ -86,7 +87,7 @@ private SrmCopyResponse srmCopy()
SRMInternalErrorException
{
TCopyFileRequest[] arrayOfFileRequests = getFileRequests(request);
long lifetime = getTotalRequestTime(request, configuration.getCopyLifetime());
long lifetime = Lifetimes.calculateLifetime(request.getDesiredTotalRequestTime(), configuration.getCopyLifetime());
String spaceToken = request.getTargetSpaceToken();

URI from_urls[] = new URI[arrayOfFileRequests.length];
Expand Down Expand Up @@ -168,30 +169,6 @@ private static TOverwriteMode getOverwriteMode(SrmCopyRequest request) throws SR
return overwriteMode;
}

private static long getTotalRequestTime(SrmCopyRequest request, long max) throws SRMInvalidRequestException
{
long lifetimeInSeconds = 0;
if (request.getDesiredTotalRequestTime() != null) {
long reqLifetime = request.getDesiredTotalRequestTime();
/* [ SRM 2.2, 5.7.2 ]
*
* o) If input parameter desiredTotalRequestTime is 0 (zero),
* each file request must be tried at least once. Negative
* value must be invalid.
*/
if (reqLifetime < 0) {
throw new SRMInvalidRequestException("Negative desiredTotalRequestTime is invalid");
}
lifetimeInSeconds = reqLifetime;
}

if (lifetimeInSeconds <= 0) {
/* FIXME: This is not spec compliant. */
return max;
}
return Math.min(TimeUnit.SECONDS.toMillis(lifetimeInSeconds), max);
}

private static TCopyFileRequest[] getFileRequests(SrmCopyRequest request) throws SRMInvalidRequestException
{
if (request.getArrayOfFileRequests() == null) {
Expand Down
29 changes: 3 additions & 26 deletions modules/srm-server/src/main/java/org/dcache/srm/handler/SrmPrepareToGet.java
View file
Expand Up @@ -6,7 +6,6 @@

import java.net.URI;
import java.util.Arrays;
import java.util.concurrent.TimeUnit;

import org.dcache.srm.AbstractStorageElement;
import org.dcache.srm.SRM;
Expand All @@ -18,6 +17,7 @@
import org.dcache.srm.scheduler.IllegalStateTransition;
import org.dcache.srm.util.Configuration;
import org.dcache.srm.util.JDC;
import org.dcache.srm.util.Lifetimes;
import org.dcache.srm.util.Tools;
import org.dcache.srm.v2_2.ArrayOfTExtraInfo;
import org.dcache.srm.v2_2.SrmPrepareToGetRequest;
Expand Down Expand Up @@ -89,7 +89,8 @@ private SrmPrepareToGetResponse srmPrepareToGet()
{
String[] protocols = getTransferProtocols(request);
String clientHost = getClientHost(request).or(this.clientHost);
long lifetime = getLifetime(request, configuration.getGetLifetime());

long lifetime = Lifetimes.calculateLifetime(request.getDesiredTotalRequestTime(), configuration.getGetLifetime());
String[] supportedProtocols = storage.supportedGetProtocols();
URI[] surls = getSurls(request);

Expand Down Expand Up @@ -159,30 +160,6 @@ private static String getExtraInfo(SrmPrepareToGetRequest request, String key)
return null;
}

private static long getLifetime(SrmPrepareToGetRequest request, long max) throws SRMInvalidRequestException
{
long lifetimeInSeconds = 0;
if (request.getDesiredTotalRequestTime() != null) {
long reqLifetime = request.getDesiredTotalRequestTime();
if (reqLifetime < 0) {
/* [ SRM 2.2, 5.2.1 ]
* m) If input parameter desiredTotalRequestTime is 0 (zero), each file request
* must be tried at least once. Negative value must be invalid.
*/
throw new SRMInvalidRequestException("Negative desiredTotalRequestTime is invalid.");
}
lifetimeInSeconds = reqLifetime;
}

if (lifetimeInSeconds > 0) {
long lifetime = TimeUnit.SECONDS.toMillis(lifetimeInSeconds);
return (lifetime > max) ? max : lifetime;
} else {
// Revisit: Behaviour doesn't match the SRM spec
return max;
}
}

private static TGetFileRequest[] getFileRequests(SrmPrepareToGetRequest request)
throws SRMInvalidRequestException
{
Expand Down
45 changes: 21 additions & 24 deletions modules/srm-server/src/main/java/org/dcache/srm/handler/SrmPrepareToPut.java
View file
Expand Up @@ -7,7 +7,6 @@

import java.net.URI;
import java.util.Arrays;
import java.util.concurrent.TimeUnit;

import org.dcache.srm.AbstractStorageElement;
import org.dcache.srm.SRM;
Expand All @@ -19,6 +18,7 @@
import org.dcache.srm.scheduler.IllegalStateTransition;
import org.dcache.srm.util.Configuration;
import org.dcache.srm.util.JDC;
import org.dcache.srm.util.Lifetimes;
import org.dcache.srm.util.Tools;
import org.dcache.srm.v2_2.ArrayOfTExtraInfo;
import org.dcache.srm.v2_2.SrmPrepareToPutRequest;
Expand Down Expand Up @@ -106,7 +106,11 @@ private SrmPrepareToPutResponse srmPrepareToPut()
}
TPutFileRequest[] fileRequests = getFileRequests(request);

long lifetime = getLifetime(request, configuration.getPutLifetime());
// assume transfers will take place in parallel
long effectiveSize = largestFileOf(fileRequests);
long lifetime = Lifetimes.calculateLifetime(request.getDesiredTotalRequestTime(),
effectiveSize, configuration.getMaximumClientAssumedBandwidth(),
configuration.getPutLifetime());
TOverwriteMode overwriteMode = getOverwriteMode(request);

String[] supportedProtocols = storage.supportedPutProtocols();
Expand Down Expand Up @@ -176,6 +180,21 @@ private SrmPrepareToPutResponse srmPrepareToPut()
}
}


private long largestFileOf(TPutFileRequest[] requests)
{
long effectiveSize = 0;

for (TPutFileRequest request : requests) {
UnsignedLong size = request.getExpectedFileSize();
if (size != null && size.longValue() > effectiveSize) {
effectiveSize = size.longValue();
}
}

return effectiveSize;
}

private static String getExtraInfo(SrmPrepareToPutRequest request, String key)
{
ArrayOfTExtraInfo storageSystemInfo = request.getStorageSystemInfo();
Expand All @@ -194,28 +213,6 @@ private static String getExtraInfo(SrmPrepareToPutRequest request, String key)
return null;
}

private static long getLifetime(SrmPrepareToPutRequest request, long max) throws SRMInvalidRequestException
{
long lifetimeInSeconds = 0;
if (request.getDesiredTotalRequestTime() != null) {
long reqLifetime = request.getDesiredTotalRequestTime();
if (reqLifetime < 0) {
/* [ SRM 2.2, 5.5.2 ]
* q) If input parameter desiredTotalRequestTime is 0 (zero), each file
* request must be tried at least once. Negative value must be invalid.
*/
throw new SRMInvalidRequestException("Negative desiredTotalRequestTime is invalid.");
}
lifetimeInSeconds = reqLifetime;
}
if (lifetimeInSeconds <= 0) {
// Revisit: Behaviour doesn't match the SRM spec
return max;
}
long lifetime = TimeUnit.SECONDS.toMillis(lifetimeInSeconds);
return lifetime > max ? max : lifetime;
}

private static TOverwriteMode getOverwriteMode(SrmPrepareToPutRequest request)
throws SRMNotSupportedException
{
Expand Down
4 changes: 3 additions & 1 deletion modules/srm-server/src/main/java/org/dcache/srm/request/CopyFileRequest.java
View file
Expand Up @@ -319,6 +319,8 @@ public void setSize(long size)
} finally {
runlock();
}

reassessLifetime(size);
}

@Override
Expand Down Expand Up @@ -463,7 +465,7 @@ private void runLocalToLocalCopy() throws IllegalStateTransition, SRMException
}
return;
}
size = fmd.size;
setSize(fmd.size);

if (getDestinationFileId() == null) {
addHistoryEvent("Doing name space lookup.");
Expand Down
17 changes: 17 additions & 0 deletions modules/srm-server/src/main/java/org/dcache/srm/request/FileRequest.java
View file
Expand Up @@ -92,6 +92,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import org.dcache.srm.scheduler.State;
import org.dcache.srm.util.Configuration;
import org.dcache.srm.util.JDC;
import org.dcache.srm.util.Lifetimes;
import org.dcache.srm.v2_2.TReturnStatus;

import static com.google.common.base.Predicates.in;
Expand Down Expand Up @@ -306,6 +307,22 @@ public QOSTicket getQOSTicket() {

public abstract long extendLifetime(long newLifetime) throws SRMException ;

protected void reassessLifetime(long fileSize)
{
long currentLifetime = getLifetime();

Configuration config = SRM.getSRM().getConfiguration();
long newLifetime = Lifetimes.calculateRequestLifetimeWithWorkaround(currentLifetime,
fileSize, config.getMaximumClientAssumedBandwidth(), config.getGetLifetime());
try {
if (newLifetime > currentLifetime) {
extendLifetime(newLifetime);
}
} catch (SRMException e) {
logger.debug("Unable to adjust lifetime: {}", e.getMessage());
}
}

@Override
public JDC applyJdc()
{
Expand Down
3 changes: 2 additions & 1 deletion modules/srm-server/src/main/java/org/dcache/srm/request/GetFileRequest.java
View file
Expand Up @@ -241,7 +241,6 @@ public String getFileId() {
}
}


@Override
public RequestFileStatus getRequestFileStatus(){
RequestFileStatus rfs;
Expand Down Expand Up @@ -575,6 +574,8 @@ private void setFileMetaData(FileMetaData fileMetaData) {
} finally {
wunlock();
}

reassessLifetime(fileMetaData.size);
}

public TReturnStatus release()
Expand Down
26 changes: 26 additions & 0 deletions modules/srm-server/src/main/java/org/dcache/srm/util/Configuration.java
View file
Expand Up @@ -84,6 +84,8 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import org.dcache.srm.SRMUserPersistenceManager;
import org.dcache.srm.client.Transport;

import static com.google.common.base.Preconditions.checkArgument;

public class Configuration {

private static final String INFINITY = "infinity";
Expand Down Expand Up @@ -159,6 +161,8 @@ public class Configuration {
protected long reserveSpaceLifetime = 24*60*60*1000;
protected long defaultSpaceLifetime = 24*60*60*1000;

protected long maximumClientAssumedBandwidth = 0;

protected boolean useUrlcopyScript=false;
protected boolean useDcapForSrmCopy=false;
protected boolean useGsiftpForSrmCopy=true;
Expand Down Expand Up @@ -713,6 +717,28 @@ public PlatformTransactionManager getTransactionManager()
return transactionManager;
}

/**
* Set the maximum allowed client-assumed bandwidth. If clients make
* requests with too short a lifetime then they are assuming a bandwidth in
* excess of this maximum. Such requests will be given longer, more
* realistic lifetimes.
* @value the bandwidth in kiB/s or zero to disable this feature.
*/
public void setMaximumClientAssumedBandwidth(long value)
{
checkArgument(value >= 0, "Bandwidth must be 0 or a positive value");
maximumClientAssumedBandwidth = value;
}

/**
* Get the maximum allowed client-assumed bandwidth.
* @return the bandwidth in kiB/s or zero if this feature is disable.
*/
public long getMaximumClientAssumedBandwidth()
{
return maximumClientAssumedBandwidth;
}

/**
* Getter for property getMaxPollPeriod.
* @return Value of property getMaxPollPeriod.
Expand Down

0 comments on commit b13fa36

Please sign in to comment.