Skip to content

Commit

Permalink
nfs: bind layout disposal to layout state id
Browse files Browse the repository at this point in the history 
Motivation:
If a client calls CLOSE without LAYOUTRETURN, then, on open-stateid
disposal dCach will issue LAYOUT_RECALL. The recall uses layout stateid,
this is can be disposed by client before open-stateid disposal is
triggered, this client will get BAD_STATEID.

Modification:
Bind layout disposal to layout stateid. The door still will trigger
layout recall, if CLOSE arrives before LAYOUTRETURN, as open-stateid
disposal will trigger layout stateid disposal.

Result:
More predictable behavior.

Acked-by: Paul Millar
Target: master, 9.0, 8.2
Require-book: no
Require-notes: yes
  • Loading branch information
@kofemann
kofemann committed Mar 29, 2023
1 parent 431d138 commit 0391098
Showing 1 changed file with 42 additions and 39 deletions.
81 changes: 42 additions & 39 deletions modules/dcache-nfs/src/main/java/org/dcache/chimera/nfsv41/door/NFSv41Door.java
View file
Expand Up @@ -724,10 +724,10 @@ public Layout layoutGet(CompoundContext context, LAYOUTGET4args args)

transfer = args.loga_iomode == layoutiomode4.LAYOUTIOMODE4_RW ?

new WriteTransfer(_pnfsHandler, client, openStateId, nfsInode,
new WriteTransfer(_pnfsHandler, client, layoutType, openStateId, nfsInode,
context.getRpcCall().getCredential().getSubject())
:
new ReadTransfer(_pnfsHandler, client, openStateId, nfsInode,
new ReadTransfer(_pnfsHandler, client, layoutType, openStateId, nfsInode,
context.getRpcCall().getCredential().getSubject());

transfer.setProtocolInfo(protocolInfo);
Expand All @@ -740,47 +740,15 @@ public Layout layoutGet(CompoundContext context, LAYOUTGET4args args)
transfer.setIoQueue(_ioQueue);
transfer.setKafkaSender(_kafkaSender);

/*
* As all our layouts marked 'return-on-close', stop mover when
* open-state disposed on CLOSE.
*/
final NfsTransfer t = transfer;
openStateId.addDisposeListener(state -> {
/*
* Cleanup transfer when state invalidated.
*/
if (t.hasMover()) {
// FIXME: as long as we can identify client's capabilities the workaround apply to flex_files layouts
// to work correctly with proxy-io. As modern rhel clients (>= 7) use flex_files by default it's ok.
if (layoutType == layouttype4.LAYOUT4_FLEX_FILES && client.isLeaseValid() && client.getCB() != null) {
/*
* Due to race in the Linux kernel client, a server might see CLOSE before
* the last WRITE operation have been processed by a data server. Thus,
* recall the layout (enforce dirty page flushing) and return a NFS4ERR_DELAY.
*
* see: https://bugzilla.redhat.com/show_bug.cgi?id=1901524
*/
_log.warn(
"Deploying work-around for buggy client {} issuing CLOSE before LAYOUT_RETURN for transfer {}@{} of {}",
t.getMoverId(), t.getPool(), t.getPnfsId(),
t.getClient().getRemoteAddress());
t.recallLayout(_callbackExecutor);
throw new DelayException("Close before layoutreturn");
} else {
_log.warn("Removing orphan mover: {}@{} for {} by {}",
t.getMoverId(), t.getPool(), t.getPnfsId(),
t.getClient().getRemoteAddress());
t.shutdownMover();
}
}
if (t.isWrite()) {
/* write request keep in the message map to
* avoid re-creates and trigger errors.
*/
_transfers.remove(openStateId.stateid());
}
});

_transfers.put(openStateId.stateid(), transfer);
} else {
// keep debug context in sync
Expand Down Expand Up @@ -1302,9 +1270,9 @@ public String toString() {

private class ReadTransfer extends NfsTransfer {

public ReadTransfer(PnfsHandler pnfs, NFS4Client client, NFS4State openStateId,
public ReadTransfer(PnfsHandler pnfs, NFS4Client client, layouttype4 layouttype, NFS4State openStateId,
Inode nfsInode, Subject ioSubject) throws ChimeraNFSException {
super(pnfs, client, openStateId, nfsInode, ioSubject);
super(pnfs, client, layouttype, openStateId, nfsInode, ioSubject);
}

@Override
Expand Down Expand Up @@ -1367,9 +1335,9 @@ public synchronized boolean hasMover() {

private class WriteTransfer extends NfsTransfer {

public WriteTransfer(PnfsHandler pnfs, NFS4Client client, NFS4State openStateId,
public WriteTransfer(PnfsHandler pnfs, NFS4Client client, layouttype4 layoutType, NFS4State openStateId,
Inode nfsInode, Subject ioSubject) throws ChimeraNFSException {
super(pnfs, client, openStateId, nfsInode, ioSubject);
super(pnfs, client, layoutType, openStateId, nfsInode, ioSubject);
}

@Override
Expand Down Expand Up @@ -1436,14 +1404,49 @@ abstract private class NfsTransfer extends RedirectedTransfer<PoolDS> {
private final NFS4Client _client;
protected boolean shutdownInProgress;

NfsTransfer(PnfsHandler pnfs, NFS4Client client, NFS4State openStateId,
NfsTransfer(PnfsHandler pnfs, NFS4Client client, layouttype4 layoutType, NFS4State openStateId,
Inode nfsInode, Subject ioSubject) throws ChimeraNFSException {
super(pnfs, Subjects.ROOT, Restrictions.none(), Subjects.fromUnixNumericSubject(ioSubject), null);

_nfsInode = nfsInode;

// layout, or a transfer in dCache language, must have a unique stateid
_stateid = client.createState(openStateId.getStateOwner(), openStateId);

/*
* As all our layouts marked 'return-on-close', stop mover when
* layout-state disposed (which will trigger by open state disposal) on CLOSE.
*/
_stateid.addDisposeListener(state -> {
/*
* Cleanup transfer when state invalidated.
*/
var t = this;
if (t.hasMover()) {
// FIXME: as long as we can identify client's capabilities the workaround apply to flex_files layouts
// to work correctly with proxy-io. As modern rhel clients (>= 7) use flex_files by default it's ok.
if (layoutType == layouttype4.LAYOUT4_FLEX_FILES && client.isLeaseValid() && client.getCB() != null) {
/*
* Due to race in the Linux kernel client, a server might see CLOSE before
* the last WRITE operation have been processed by a data server. Thus,
* recall the layout (enforce dirty page flushing) and return a NFS4ERR_DELAY.
*
* see: https://bugzilla.redhat.com/show_bug.cgi?id=1901524
*/
_log.warn(
"Deploying work-around for buggy client {} issuing CLOSE before LAYOUT_RETURN for transfer {}@{} of {}",
t.getMoverId(), t.getPool(), t.getPnfsId(),
t.getClient().getRemoteAddress());
t.recallLayout(_callbackExecutor);
throw new DelayException("Close before layoutreturn");
} else {
_log.warn("Removing orphan mover: {}@{} for {} by {}",
t.getMoverId(), t.getPool(), t.getPnfsId(),
t.getClient().getRemoteAddress());
t.shutdownMover();
}
}
});
_openStateid = openStateId;
_client = client;
}
Expand Down

0 comments on commit 0391098

Please sign in to comment.