Skip to content

Commit

Permalink
poolmanager: refactor stage check, fix some staging decisions
Browse files Browse the repository at this point in the history
Motivation:

The algorithm to check whether a file can be staged (i.e., file is
stored on tape and poolmanager is configured to support staging) appears
in several places. This violates the DRY principle.

There are a few places where the FSM will attempt to stage a file even
if that file isn't stored on tape:

  * If the initial check returns NOT_PERMITTED (replicas exist, but not
    accessible via any read-link selected for this request), staging is
    enabled and pool-to-pool transfers disabled.

  * If the initial check returns COST_EXCEEDED (only under classic
    partition, with cost exceeding cost-cut but without exceeding panic
    cost-cut), p2pOnCost is disabled, and both staging and stageOnCode
    are enabled.

In both cases, FSM will attempt to stage files it knows are not stored
on tape.

(There is another, egregious place where FSM attempts to stage a
non-tape file. This will be fixed in a subsequent patch.)

This is bad as information the original error is lost when it is
replaced with a generic (and unhelpful) staging error message.

Modification:

Rename an existing method (`canStage --> isStagingAllowed`) to better
reflect that this method is an authorisation check.

Create method to describe whether a file is stageable
(`isFileStageable`).  To be stageable a file must be stored on tape and
poolmanager is configured to allow staging.

For the two cases where FSM will attempt to stage files that are not
stored on tape, this patch updates FSM to behave the same way for
non-tape files, whether or not staging is enabled.

In the NOT_PERMITTED case, with this patch FSM will attempt a
pool-to-pool (despite p2p being disabled).

In the COST_EXCEEDED case, with this patch FSM will fail the request.

Result:

No user observable changes.

In some cases, poolmanager will report more useful error messages
instead of the generic "No pool candidates available/configured/left for
stage" message.

Target: master
Requires-notes: yes
Requires-book: no
Patch: https://rb.dcache.org/r/12815/
Acked-by: Tigran Mkrtchyan
  • Loading branch information
paulmillar committed Feb 15, 2021
1 parent a90be72 commit 5faf380
Showing 1 changed file with 14 additions and 9 deletions.
Expand Up @@ -1307,7 +1307,12 @@ private void stateLoop() {
}
}

private boolean canStage()
private boolean isFileStageable()
{
return _parameter._hasHsmBackend && _storageInfo.isStored();
}

private boolean isStagingAllowed()
{
/* If the result is cached or the door disabled staging,
* then we don't check the permissions.
Expand Down Expand Up @@ -1348,7 +1353,7 @@ private void nextStep(RequestState state) {
return;
}

if (state == RequestState.ST_STAGE && !canStage()) {
if (state == RequestState.ST_STAGE && !isStagingAllowed()) {
_state = RequestState.ST_DONE;
updateStatus("Failed: stage not allowed");
_log.debug("Subject is not authorized to stage");
Expand Down Expand Up @@ -1518,7 +1523,7 @@ private void stateEngine(Object inputObject) {

case NOT_FOUND:
_log.debug(" stateEngine: RequestStatusCode.NOT_FOUND ");
if (_parameter._hasHsmBackend && _storageInfo.isStored()) {
if (isFileStageable()) {
_log.debug(" stateEngine: parameter has HSM backend and the file is stored on tape ");
nextStep(RequestState.ST_STAGE);
} else {
Expand All @@ -1541,14 +1546,14 @@ private void stateEngine(Object inputObject) {
//
// if we don't have an hsm we overwrite the p2pAllowed
//
nextStep(_parameter._p2pAllowed || ! _parameter._hasHsmBackend
nextStep(_parameter._p2pAllowed || !isFileStageable()
? RequestState.ST_POOL_2_POOL : RequestState.ST_STAGE);
break;

case COST_EXCEEDED:
if (_parameter._p2pOnCost) {
nextStep(RequestState.ST_POOL_2_POOL);
} else if (_parameter._hasHsmBackend && _parameter._stageOnCost) {
} else if (isFileStageable() && _parameter._stageOnCost) {
nextStep(RequestState.ST_STAGE);
} else {
failRequest(127, "Cost exceeded (st,p2p not allowed)");
Expand Down Expand Up @@ -1580,7 +1585,7 @@ private void stateEngine(Object inputObject) {
if (_bestPool == null) {
if (_enforceP2P) {
failRequest(_currentRc, _currentRm);
} else if (_parameter._hasHsmBackend && _storageInfo.isStored()) {
} else if (isFileStageable()) {
_log.info("ST_POOL_2_POOL : Pool to pool not permitted, trying to stage the file");
nextStep(RequestState.ST_STAGE);
} else {
Expand All @@ -1595,7 +1600,7 @@ private void stateEngine(Object inputObject) {
case S_COST_EXCEEDED:
_log.info("ST_POOL_2_POOL : RequestStatusCode.S_COST_EXCEEDED");

if (_parameter._hasHsmBackend && _parameter._stageOnCost && _storageInfo.isStored()) {
if (isFileStageable() && _parameter._stageOnCost) {
if (_enforceP2P) {
failRequest(_currentRc, _currentRm);
} else {
Expand Down Expand Up @@ -1634,7 +1639,7 @@ private void stateEngine(Object inputObject) {
default:
if (_enforceP2P) {
failRequest(_currentRc, _currentRm);
} else if (_parameter._hasHsmBackend && _storageInfo.isStored()) {
} else if (isFileStageable()) {
nextStep(RequestState.ST_STAGE);
} else {
// FIXME refactor askForPoolToPool to avoid
Expand Down Expand Up @@ -1689,7 +1694,7 @@ private void stateEngine(Object inputObject) {
}
} else {
_log.info("ST_POOL_2_POOL : Pool to pool reported a problem");
if (_parameter._hasHsmBackend && _storageInfo.isStored()) {
if (isFileStageable()) {
_log.info("ST_POOL_2_POOL : trying to stage the file");
nextStep(RequestState.ST_STAGE);
} else {
Expand Down

0 comments on commit 5faf380

Please sign in to comment.