Skip to content

Commit

Permalink
nfs: fix COMMIT operation sent to pools
Browse files Browse the repository at this point in the history 
Motivation:
with introduction of inumber nfs file handle does not encode pnfsid any
more. We need to use file handle to find a matching mover on COMMIT.

Modification:
Use file handle to find matching mover. Remove extra map of active write
movers.

Result:
functional COMMIT on pools.

Acked-by: Paul Millar
Target: master
Require-book: no
Require-notes: no
  • Loading branch information
@kofemann
kofemann committed Oct 18, 2017
1 parent 5f08bd7 commit 84f61ce
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 74 deletions.
75 changes: 21 additions & 54 deletions modules/dcache-nfs/src/main/java/org/dcache/chimera/nfsv41/mover/EDSOperationCOMMIT.java
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
package org.dcache.chimera.nfsv41.mover;

import diskCacheV111.util.CacheException;
import diskCacheV111.util.PnfsId;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.Map;
import org.dcache.chimera.FsInodeType;
import org.dcache.nfs.ChimeraNFSException;
import org.dcache.nfs.nfsstat;
import org.dcache.nfs.status.BadHandleException;
import org.dcache.nfs.status.NfsIoException;
import org.dcache.nfs.v4.AbstractNFSv4Operation;
import org.dcache.nfs.v4.CompoundContext;
import org.dcache.nfs.v4.xdr.COMMIT4res;
Expand All @@ -19,60 +12,34 @@
import org.dcache.nfs.v4.xdr.nfs_resop4;
import org.dcache.nfs.vfs.Inode;
import org.dcache.pool.repository.RepositoryChannel;
import org.dcache.xdr.OncRpcException;


public class EDSOperationCOMMIT extends AbstractNFSv4Operation {

private final Map<PnfsId, NfsMover> _activeWrites;
private final NFSv4MoverHandler _moverHandler;

public EDSOperationCOMMIT(nfs_argop4 args, Map<PnfsId, NfsMover> activeMovers) {
public EDSOperationCOMMIT(nfs_argop4 args, NFSv4MoverHandler moverHandler) {
super(args, nfs_opnum4.OP_COMMIT);
_activeWrites = activeMovers;
_moverHandler = moverHandler;
}

@Override
public void process(CompoundContext cc, nfs_resop4 result) throws ChimeraNFSException, IOException, OncRpcException {

try {
Inode inode = cc.currentInode();
PnfsId pnfsId = toPnfsid(inode);
NfsMover mover = _activeWrites.get(pnfsId);
if (mover == null) {
throw new BadHandleException("can't find mover for pnfsid : " + pnfsId);
}

RepositoryChannel fc = mover.getMoverChannel();
fc.sync();
mover.commitFileSize(fc.size());
final COMMIT4res res = result.opcommit;
res.status = nfsstat.NFS_OK;
res.resok4 = new COMMIT4resok();
res.resok4.writeverf = mover.getBootVerifier();
} catch (CacheException e) {
throw new NfsIoException(e.getMessage());
}
}

/*
* this is shameless light copy-paste form JdbcFs.
*/
private static PnfsId toPnfsid(Inode inode) throws ChimeraNFSException {
ByteBuffer b = ByteBuffer.wrap(inode.getFileId());
int fsid = b.get();
int type = b.get();
FsInodeType inodeType = FsInodeType.valueOf(type);
if (inodeType != FsInodeType.INODE) {
throw new BadHandleException("Not a regular pnfsid");
}

int idLen = b.get();
byte[] id = new byte[idLen];
b.get(id);
int opaqueLen = b.get();
if (opaqueLen > b.remaining()) {
throw new BadHandleException("Bad pnfsid size");
}
return new PnfsId(id);
public void process(CompoundContext cc, nfs_resop4 result) throws ChimeraNFSException, IOException {

final COMMIT4res res = result.opcommit;
Inode inode = cc.currentInode();
/**
* The nfs commit operation comes without a stateid. The assumption, is
* that for now we have only one writer and, as a result, pnfsid will
* point only to a single mover.
*/
NfsMover mover = _moverHandler.getPnfsIdByHandle(inode.toNfsHandle());

RepositoryChannel fc = mover.getMoverChannel();
fc.sync();
mover.commitFileSize(fc.size());

res.status = nfsstat.NFS_OK;
res.resok4 = new COMMIT4resok();
res.resok4.writeverf = mover.getBootVerifier();
}
}
32 changes: 15 additions & 17 deletions modules/dcache-nfs/src/main/java/org/dcache/chimera/nfsv41/mover/NFSv4MoverHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@

import java.io.IOException;
import java.net.InetSocketAddress;
import java.nio.file.StandardOpenOption;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import diskCacheV111.util.PnfsId;
import java.util.Arrays;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
Expand All @@ -23,6 +20,7 @@
import org.dcache.auth.Subjects;
import org.dcache.cells.CellStub;
import org.dcache.nfs.ChimeraNFSException;
import org.dcache.nfs.status.BadHandleException;
import org.dcache.nfs.v3.xdr.nfs3_prot;
import org.dcache.nfs.v4.AbstractNFSv4Operation;
import org.dcache.nfs.v4.NFSServerV41;
Expand All @@ -45,7 +43,6 @@
import org.dcache.nfs.v4.xdr.nfs_opnum4;
import org.dcache.nfs.v4.xdr.nfsace4;
import org.dcache.nfs.v4.xdr.stateid4;
import org.dcache.nfs.vfs.DirectoryEntry;
import org.dcache.nfs.vfs.FsStat;
import org.dcache.nfs.vfs.Inode;
import org.dcache.nfs.vfs.Stat;
Expand Down Expand Up @@ -205,12 +202,6 @@ public NfsIdMapping getIdMapper() {
private final Map<stateid4, NfsMover> _activeIO = new ConcurrentHashMap<>();
private final NFSv4OperationFactory _operationFactory = new EDSNFSv4OperationFactory();

/**
* for The nfs commit operation comes without a stateid. We need a different way to
* wind corresponding mover. The assumption, is that nor now we have only one writer and,
* as a result, pnfsid will point only to a single mover.
*/
private final Map<PnfsId, NfsMover> _activeWrites = new ConcurrentHashMap<>();
private final NFSServerV41 _embededDS;
private final EmbeddedV3 _v3;

Expand Down Expand Up @@ -274,9 +265,6 @@ public NFSv4MoverHandler(PortRange portRange, IoStrategy ioStrategy,
public void add(NfsMover mover) {
_log.debug("registering new mover {}", mover);
_activeIO.put(mover.getStateId(), mover );
if (mover.getIoMode().contains(StandardOpenOption.WRITE)) {
_activeWrites.put(mover.getFileAttributes().getPnfsId(), mover);
}
}

/**
Expand All @@ -287,9 +275,6 @@ public void add(NfsMover mover) {
public void remove(NfsMover mover) {
_log.debug("un-removing io handler for stateid {}", mover);
_activeIO.remove(mover.getStateId());
if (mover.getIoMode().contains(StandardOpenOption.WRITE)) {
_activeWrites.remove(mover.getFileAttributes().getPnfsId());
}
}

private class EDSNFSv4OperationFactory implements NFSv4OperationFactory {
Expand All @@ -299,7 +284,7 @@ public AbstractNFSv4Operation getOperation(nfs_argop4 op) {

switch (op.argop) {
case nfs_opnum4.OP_COMMIT:
return new EDSOperationCOMMIT(op, _activeWrites);
return new EDSOperationCOMMIT(op, NFSv4MoverHandler.this);
case nfs_opnum4.OP_GETATTR:
return new OperationGETATTR(op);
case nfs_opnum4.OP_PUTFH:
Expand Down Expand Up @@ -351,6 +336,19 @@ NfsMover getOrCreateMover(InetSocketAddress remoteAddress, stateid4 stateid, byt
return mover;
}

/**
* Find a mover for a corresponding nfs handle.
* @param fh file handle
* @return a mover for a given nfs file handle
* @throws ChimeraNFSException
*/
NfsMover getPnfsIdByHandle(byte[] fh) throws BadHandleException {
return _activeIO.values().stream()
.filter(m -> Arrays.equals(fh, m.getNfsFilehandle()))
.findAny()
.orElseThrow(() -> new BadHandleException("No mover for found for given file handle"));
}

/**
* Get TCP port number used by handler.
* @return port number.
Expand Down
12 changes: 9 additions & 3 deletions modules/dcache-nfs/src/main/java/org/dcache/chimera/nfsv41/mover/NfsMover.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@

import dmg.cells.nucleus.CellPath;

import org.dcache.nfs.ChimeraNFSException;
import org.dcache.nfs.v4.NFS4State;
import org.dcache.nfs.v4.NFSv41Session;
import org.dcache.nfs.v4.StateOwner;
import org.dcache.nfs.v4.xdr.stateid4;
import org.dcache.nfs.v4.xdr.verifier4;
import org.dcache.nfs.status.NfsIoException;
import org.dcache.pool.classic.Cancellable;
import org.dcache.pool.classic.ChecksumModule;
import org.dcache.pool.movers.MoverChannel;
Expand Down Expand Up @@ -154,9 +156,13 @@ protected void dispose() {
}
}

public void commitFileSize(long size) throws CacheException {
_namespace.setFileAttributes(getFileAttributes().getPnfsId(),
FileAttributes.ofSize(size));
public void commitFileSize(long size) throws ChimeraNFSException {
try {
_namespace.setFileAttributes(getFileAttributes().getPnfsId(),
FileAttributes.ofSize(size));
} catch (CacheException e) {
throw new NfsIoException("Failed to update file size in the namespace", e);
}
}

public verifier4 getBootVerifier() {
Expand Down

0 comments on commit 84f61ce

Please sign in to comment.