chimera: fix get locality to work with id-type FsInode

The current implementation of ".(get)(filename)(locality)" relies on an FsInode type which embeds the filename and the "locality" argument.  Since the filename is arbitrarily long, the encoded handle length could exceed the file handle length specified by NFS4.

The solution is to change the handling of this command (file) to use the id FsInode type and to encode the operation directly into it.  The id (pnfsid) is of fixed length, as is the code for the op.

A new FsInode type (PLOC) has been created for this purpose.  Several auxiliary methods and private classes which are no longer necessary have also been eliminated.

Testing Done:

The new code allows this operation to work successfully with both v4 and with the smaller handle size of v3 as well (both were tested).

Target: master
Request: 2.7
Patch: http://rb.dcache.org/r/6404
Require-book: no
Require-notes: yes
Acked-by: Tigran

RELEASE NOTES:  Fixes a bug in the ".(get)(filename)(locality)" preventing successful completion of the call when the filename is very long.

modules/chimera/src/main/java/org/dcache/chimera/FsInodeType.java

@@ -27,7 +27,8 @@ public enum FsInodeType {
     NAMEOF(6),   // the content of the inode is the name of the inode
     PGET(7),     // the content of the inode is the value of requested attributes
     PSET(8),     // by updating mtime of the inode the the defined attribute value is updated
-    CONST(9);    // the content of the inode is a free form information
+    CONST(9),    // the content of the inode is a free form information
+    PLOC(10);     // the content of the inode is the value of requested attributes
 
     private final int _id;
 

modules/chimera/src/main/java/org/dcache/chimera/FsInode_PLOC.java

@@ -0,0 +1,69 @@
+/*
+ * This library is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Library General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this program (see the file COPYING.LIB for more
+ * details); if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+package org.dcache.chimera;
+
+import org.dcache.chimera.posix.Stat;
+
+/**
+ * This class retrieves file locality metadata via an invocation of
+ * the pool manager.
+ *
+ * @author arossi
+ */
+public class FsInode_PLOC extends FsInode {
+    private String _locality;
+
+    public FsInode_PLOC(FileSystemProvider fs, String id) {
+        super(fs, id, FsInodeType.PLOC);
+    }
+
+    @Override
+    public int read(long pos, byte[] data, int offset, int len) {
+        String output = _locality == null? "\n" : _locality;
+
+        byte[] b = output.getBytes();
+
+        if (pos > b.length) {
+            return 0;
+        }
+
+        int copyLen = Math.min(len, b.length - (int) pos);
+        System.arraycopy(b, (int) pos, data, 0, copyLen);
+
+        return copyLen;
+    }
+
+
+    public void setLocality(String locality) {
+        _locality = locality + "\n";
+    }
+
+    @Override
+    public Stat stat() throws ChimeraFsException {
+        Stat ret = super.stat();
+        ret.setMode((ret.getMode() & 0000777) | UnixPermission.S_IFREG);
+        ret.setSize(_locality == null ? 0 : _locality.length());
+        // invalidate NFS cache
+        ret.setMTime(System.currentTimeMillis());
+        return ret;
+    }
+
+    @Override
+    public int write(long pos, byte[] data, int offset, int len) {
+        return -1;
+    }
+}

modules/chimera/src/main/java/org/dcache/chimera/JdbcFs.java

@@ -1062,12 +1062,20 @@ public FsInode inodeOf(FsInode parent, String name) throws ChimeraFsException {
                     throw new FileNotFoundHimeraFsException(name);
                 }
 
+                if (cmd[2].equals("locality")) {
+                    inode = inodeOf(parent, cmd[1]);
+                    if (!inode.exists()) {
+                        throw new FileNotFoundHimeraFsException(name);
+                    }
+                    return getPLOC(inode.toString());
+                }
+
                 /*
                  * pass in the name too (args 1 to n)
                  */
                 String[] args = new String[cmd.length - 1];
                 System.arraycopy(cmd, 1, args, 0, args.length);
-                inode = getPGET(parent, args);
+                inode = new FsInode_PGET(this, parent.toString(), args);
                 if (!inode.exists()) {
                     throw new FileNotFoundHimeraFsException(name);
                 }
@@ -2801,8 +2809,13 @@ FsInode inodeFromBytesNew(byte[] handle) throws ChimeraFsException {
                 break;
 
             case PGET:
-                inode = getPGET(inodeId, getArgs(opaque));
+                inode = new FsInode_PGET(this, inodeId, getArgs(opaque));
                 break;
+
+            case PLOC:
+                inode = getPLOC(inodeId);
+                break;
+
             default:
                 throw new FileNotFoundHimeraFsException("Unsupported file handle type: " + inodeType);
         }
@@ -2898,7 +2911,12 @@ FsInode inodeFromBytesOld(byte[] handle) throws ChimeraFsException {
                     for (int i = 0; i < argc; i++) {
                         args[i] = st.nextToken();
                     }
-                    inode = getPGET(id, args);
+                    inode = new FsInode_PGET(this, id, args);
+                    break;
+
+                case PLOC:
+                    id = st.nextToken();
+                    inode = getPLOC(id);
                     break;
 
             }
@@ -2916,18 +2934,12 @@ public byte[] inodeToBytes(FsInode inode) throws ChimeraFsException {
     }
 
     /**
-     * So that subclasses can do something different (like caching).
-     */
-    protected FsInode_PGET getPGET(String id, String[] args)
-                    throws ChimeraFsException {
-        return new FsInode_PGET(this, id, args);
-    }
-
-    /**
-     * So that subclasses can do something different (like caching).
+     * Subclass should cache the inode object for proper handling.
+     * This implementation will not work correctly by itself.  The
+     * adapter here is a placeholder.
      */
-    protected FsInode_PGET getPGET(FsInode parent, String[] args)
+    protected FsInode_PLOC getPLOC(String id)
                     throws ChimeraFsException {
-        return new FsInode_PGET(this, parent.toString(), args);
+        return new FsInode_PLOC(this, id);
     }
 }

modules/dcache-chimera/src/main/java/org/dcache/chimera/DCacheAwareJdbcFs.java

@@ -68,7 +68,6 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
-import java.util.Arrays;
 import java.util.EnumSet;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
@@ -89,80 +88,28 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 /**
  * Overrides protected methods so as to be able to provide live locality
  * information if requested. Embeds a Guava cache to maintain already
- * initialized PGET nodes; the latter is calls out in turn to the PNFS and Pool
+ * initialized PLOC nodes; the latter is calls out in turn to the PNFS and Pool
  * managers.
  *
  * @author arossi
  */
 public class DCacheAwareJdbcFs extends JdbcFs {
-
-    private final class LocalityArgs {
-        private final FsInode parent;
-        private final String id;
-        private final String[] args;
-
-        private LocalityArgs(FsInode parent, String[] args) {
-            this.parent = parent;
-            this.id = parent.toString();
-            this.args = args.clone();
-        }
-
-        private LocalityArgs(JdbcFs fs, String id, String[] args) {
-            this.parent = new FsInode(fs, id);
-            this.id = id;
-            this.args = args.clone();
-        }
-
-        public boolean equals(Object o) {
-            if (this == o) {
-                return true;
-            }
-
-            if (!(o instanceof LocalityArgs)) {
-                return false;
-            }
-
-            LocalityArgs largs = (LocalityArgs) o;
-
-            if (!id.equals(largs.id)) {
-                return false;
-            }
-
-            return Arrays.equals(args, largs.args);
-        }
-
-        public int hashCode() {
-            return toString().hashCode();
-        }
-
-        public String toString() {
-            return id + Arrays.asList(args);
-        }
-
-    }
-
     private final class LocalityLoader extends
-                    CacheLoader<LocalityArgs, FsInode_PGET> {
+                    CacheLoader<String, FsInode_PLOC> {
 
         @Override
-        public FsInode_PGET load(LocalityArgs key) throws Exception {
-            FsInode_PGET pget = getSuperPGET(key.parent, key.args);
-            if (pget.hasMetadataFor(LOCALITY)) {
-                FsInode pathInode = inodeOf(key.parent, pget.getName());
-                String path = inode2path(pathInode);
-                String locality = getFileLocality(path);
-                pget.setMetadata(LOCALITY, locality);
-                /*
-                 * need to override the cached stat value
-                 */
-                pget.stat();
-            }
-            return pget;
+        public FsInode_PLOC load(String id) throws Exception {
+            FsInode_PLOC ploc = getSuperPLOC(id);
+            FsInode pathInode = new FsInode(DCacheAwareJdbcFs.this, id);
+            ploc.setLocality(getFileLocality(inode2path(pathInode)));
+            /*
+             * need to override the cached stat value
+             */
+            ploc.stat();
+            return ploc;
         }
     }
 
-    private static final String LOCALITY = "locality";
-
     private static String host;
 
     static {
@@ -181,7 +128,7 @@ public FsInode_PGET load(LocalityArgs key) throws Exception {
      * write because we do not want to get too far out of synchronization with
      * the live system. Maintains entries for 1 minute.
      */
-    private final LoadingCache<LocalityArgs, FsInode_PGET> CACHE
+    private final LoadingCache<String, FsInode_PLOC> CACHE
         = CacheBuilder.newBuilder().expireAfterWrite(1, TimeUnit.MINUTES)
                                    .maximumSize(64)
                                    .softValues().build(new LocalityLoader());
@@ -205,26 +152,10 @@ public void setPoolManagerStub(CellStub poolManagerStub) {
     /**
      * Goes to the cache instead of directly creating the inode.
      */
-    protected FsInode_PGET getPGET(FsInode parent, String[] cmd)
-                    throws ChimeraFsException {
-        try {
-            return CACHE.get(new LocalityArgs(parent, cmd));
-        } catch (ExecutionException t) {
-            Throwable cause = t.getCause();
-            if (cause instanceof ChimeraFsException) {
-                throw (ChimeraFsException)cause;
-            }
-            throw new ChimeraFsException(t.toString());
-        }
-    }
-
-    /**
-     * Goes to the cache instead of directly creating the inode.
-     */
-    protected FsInode_PGET getPGET(String id, String[] cmd)
+    protected FsInode_PLOC getPLOC(String id)
                     throws ChimeraFsException {
         try {
-            return CACHE.get(new LocalityArgs(this, id, cmd));
+            return CACHE.get(id);
         } catch (ExecutionException t) {
             Throwable cause = t.getCause();
             if (cause instanceof ChimeraFsException) {
@@ -262,8 +193,8 @@ private String getFileLocality(String filePath) throws IOException {
         return locality.toString();
     }
 
-    private FsInode_PGET getSuperPGET(FsInode parent, String[] cmd)
+    private FsInode_PLOC getSuperPLOC(String id)
                     throws ChimeraFsException {
-        return super.getPGET(parent, cmd);
+        return super.getPLOC(id);
     }
 }