Skip to content

Commit

Permalink
dcache-bulk: fix missing pnfsid on cancel of PIN/STAGE
Browse files Browse the repository at this point in the history
Motivation:

The "prestore" option on bulk requests is there to allow
greater throughput by fetching file attributes for targets
lazily rather than at the moment of request submission.
This means that without prestore, file targets of the request
will not have, among other things, their pnfsid defined or
stored in the database. (This does not apply to "discovered"
paths, i.e., through directory recursion, as the directory
listing automatically provides the extra attributes.)

This situation, however, means than if the target is
cancelled, its pnfsid will not be available; this is
only important for the PIN/STAGE activities, since
they attempt to issue an unpin request, and that
message requires a pnfsid.

Issuing a cancel request on a PIN request not
marked with `-prestore=true` will in fact raise
an NPE because of the precondition check in
the unpin message constructor; and this in turn
leaves the original request stuck in the CANCELLING
state.

Modification:

In the case of cancel, fetch the missing pnfsid.
As stated in the GitHub issue, the throughput
considerations can be ignored in this case.

Result:

Cancellation of PIN requests which were not
marked with `-prestore=true` no longer fail
to complete, and the unpin request successfully
arrives at PinManager.

Target: master
Request: 8.2
Patch: https://rb.dcache.org/r/13715/
Closes: #6823
Bug: #6823
Requires-notes: yes
Requires-book: no
Acked-by: Tigran
  • Loading branch information
alrossi committed Oct 17, 2022
1 parent dc54fb8 commit 4c52046
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.dcache.pinmanager.PinManagerPinMessage;
import org.dcache.services.bulk.BulkServiceException;
import org.dcache.services.bulk.util.BulkRequestTarget;
import org.dcache.vehicles.FileAttributes;

Expand All @@ -90,7 +91,12 @@ public PinActivity(String name, TargetType targetType) {

public void cancel(BulkRequestTarget target) {
super.cancel(target);
pinManager.send(unpinMessage(id, target));
try {
pinManager.send(unpinMessage(id, target));
} catch (CacheException e) {
target.setErrorObject(new BulkServiceException("unable to fetch pnfsid of target in "
+ "order to cancel pinning.", e));
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING

abstract class PinManagerActivity extends BulkActivity<Message> implements PinManagerAware,
NamespaceHandlerAware {

protected CellStub pinManager;
protected PnfsHandler pnfsHandler;

Expand Down Expand Up @@ -122,10 +121,13 @@ protected FileAttributes getAttributes(FsPath path) throws CacheException {
return pnfsHandler.getFileAttributes(path, MINIMALLY_REQUIRED_ATTRIBUTES);
}

protected PinManagerUnpinMessage unpinMessage(String id, BulkRequestTarget target) {
PinManagerUnpinMessage message = new PinManagerUnpinMessage(target.getPnfsId());
message.setRequestId(id);
return message;
protected PinManagerUnpinMessage unpinMessage(String id, BulkRequestTarget target)
throws CacheException {
PnfsId pnfsId = target.getPnfsId();
if (pnfsId == null) {
pnfsId = getAttributes(target.getPath()).getPnfsId();
}
return unpinMessage(id, pnfsId);
}

protected PinManagerUnpinMessage unpinMessage(String id, PnfsId pnfsId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.dcache.pinmanager.PinManagerPinMessage;
import org.dcache.services.bulk.BulkServiceException;
import org.dcache.services.bulk.util.BulkRequestTarget;
import org.dcache.vehicles.FileAttributes;
import org.json.JSONObject;
Expand All @@ -99,7 +100,12 @@ public StageActivity(String name, TargetType targetType) {

public void cancel(BulkRequestTarget target) {
super.cancel(target);
pinManager.send(unpinMessage(id, target));
try {
pinManager.send(unpinMessage(id, target));
} catch (CacheException e) {
target.setErrorObject(new BulkServiceException("unable to fetch pnfsid of target in "
+ "order to cancel staging.", e));
}
}

@Override
Expand Down

0 comments on commit 4c52046

Please sign in to comment.