hadoop-hdfs-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cmcc...@apache.org
Subject svn commit: r1539839 [1/2] - in /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs: ./ src/main/java/org/apache/hadoop/hdfs/ src/main/java/org/apache/hadoop/hdfs/protocol/ src/main/java/org/apache/hadoop/hdfs/protocolPB/ src/main/java/org/apache/hado...
Date Thu, 07 Nov 2013 22:07:17 GMT
Author: cmccabe
Date: Thu Nov  7 22:07:16 2013
New Revision: 1539839

URL: http://svn.apache.org/r1539839
Log:
HDFS-5326. add modifyDirective to cacheAdmin (cmccabe)

Removed:
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/AddPathBasedCacheDirectiveException.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheDescriptor.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/RemovePathBasedCacheDescriptorException.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/RemovePathBasedCacheEntryException.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestClientNamenodeProtocolServerSideTranslatorPB.java
Modified:
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheDirective.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheEntry.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolServerSideTranslatorPB.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolTranslatorPB.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CacheManager.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOpCodes.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/CacheAdmin.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/XMLUtils.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/ClientNamenodeProtocol.proto
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/OfflineEditsViewerHelper.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNamenodeRetryCache.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestPathBasedCacheRequests.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestRetryCacheWithHA.java
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored
    hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt Thu Nov  7 22:07:16 2013
@@ -188,6 +188,8 @@ Trunk (Unreleased)
     HDFS-5386. Add feature documentation for datanode caching.
     (Colin Patrick McCabe via cnauroth)
 
+    HDFS-5326. add modifyDirective to cacheAdmin.  (cmccabe)
+
   OPTIMIZATIONS
     HDFS-5349. DNA_CACHE and DNA_UNCACHE should be by blockId only. (cmccabe)
 

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java Thu Nov  7 22:07:16 2013
@@ -117,7 +117,6 @@ import org.apache.hadoop.hdfs.protocol.D
 import org.apache.hadoop.hdfs.protocol.ExtendedBlock;
 import org.apache.hadoop.hdfs.protocol.HdfsBlocksMetadata;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants;
-import org.apache.hadoop.hdfs.protocol.PathBasedCacheDescriptor;
 import org.apache.hadoop.hdfs.protocol.PathBasedCacheDirective;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants.DatanodeReportType;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction;
@@ -2291,7 +2290,7 @@ public class DFSClient implements java.i
     }
   }
 
-  public PathBasedCacheDescriptor addPathBasedCacheDirective(
+  public long addPathBasedCacheDirective(
       PathBasedCacheDirective directive) throws IOException {
     checkOpen();
     try {
@@ -2301,21 +2300,31 @@ public class DFSClient implements java.i
     }
   }
   
-  public void removePathBasedCacheDescriptor(long id)
+  public void modifyPathBasedCacheDirective(
+      PathBasedCacheDirective directive) throws IOException {
+    checkOpen();
+    try {
+      namenode.modifyPathBasedCacheDirective(directive);
+    } catch (RemoteException re) {
+      throw re.unwrapRemoteException();
+    }
+  }
+
+  public void removePathBasedCacheDirective(long id)
       throws IOException {
     checkOpen();
     try {
-      namenode.removePathBasedCacheDescriptor(id);
+      namenode.removePathBasedCacheDirective(id);
     } catch (RemoteException re) {
       throw re.unwrapRemoteException();
     }
   }
   
-  public RemoteIterator<PathBasedCacheDescriptor> listPathBasedCacheDescriptors(
-      String pool, String path) throws IOException {
+  public RemoteIterator<PathBasedCacheDirective> listPathBasedCacheDirectives(
+      PathBasedCacheDirective filter) throws IOException {
     checkOpen();
     try {
-      return namenode.listPathBasedCacheDescriptors(0, pool, path);
+      return namenode.listPathBasedCacheDirectives(0, filter);
     } catch (RemoteException re) {
       throw re.unwrapRemoteException();
     }

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java Thu Nov  7 22:07:16 2013
@@ -211,9 +211,9 @@ public class DFSConfigKeys extends Commo
   public static final String  DFS_NAMENODE_LIST_CACHE_POOLS_NUM_RESPONSES =
       "dfs.namenode.list.cache.pools.num.responses";
   public static final int     DFS_NAMENODE_LIST_CACHE_POOLS_NUM_RESPONSES_DEFAULT = 100;
-  public static final String  DFS_NAMENODE_LIST_CACHE_DESCRIPTORS_NUM_RESPONSES =
-      "dfs.namenode.list.cache.descriptors.num.responses";
-  public static final int     DFS_NAMENODE_LIST_CACHE_DESCRIPTORS_NUM_RESPONSES_DEFAULT = 100;
+  public static final String  DFS_NAMENODE_LIST_CACHE_DIRECTIVES_NUM_RESPONSES =
+      "dfs.namenode.list.cache.directives.num.responses";
+  public static final int     DFS_NAMENODE_LIST_CACHE_DIRECTIVES_NUM_RESPONSES_DEFAULT = 100;
   public static final String  DFS_NAMENODE_PATH_BASED_CACHE_REFRESH_INTERVAL_MS =
       "dfs.namenode.path.based.cache.refresh.interval.ms";
   public static final long    DFS_NAMENODE_PATH_BASED_CACHE_REFRESH_INTERVAL_MS_DEFAULT = 300000L;

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java Thu Nov  7 22:07:16 2013
@@ -68,7 +68,6 @@ import org.apache.hadoop.hdfs.protocol.H
 import org.apache.hadoop.hdfs.protocol.HdfsLocatedFileStatus;
 import org.apache.hadoop.hdfs.protocol.LocatedBlock;
 import org.apache.hadoop.hdfs.protocol.PathBasedCacheDirective;
-import org.apache.hadoop.hdfs.protocol.PathBasedCacheDescriptor;
 import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport;
 import org.apache.hadoop.hdfs.protocol.SnapshottableDirectoryStatus;
 import org.apache.hadoop.hdfs.security.token.block.InvalidBlockTokenException;
@@ -82,6 +81,7 @@ import org.apache.hadoop.security.token.
 import org.apache.hadoop.util.Progressable;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 
 
 /****************************************************************
@@ -1586,57 +1586,74 @@ public class DistributedFileSystem exten
   /**
    * Add a new PathBasedCacheDirective.
    * 
-   * @param directive A PathBasedCacheDirectives to add
-   * @return PathBasedCacheDescriptor associated with the added directive
+   * @param directive A directive to add.
+   * @return the ID of the directive that was created.
    * @throws IOException if the directive could not be added
    */
-  public PathBasedCacheDescriptor addPathBasedCacheDirective(
+  public long addPathBasedCacheDirective(
       PathBasedCacheDirective directive) throws IOException {
+    Preconditions.checkNotNull(directive.getPath());
     Path path = new Path(getPathName(fixRelativePart(directive.getPath()))).
         makeQualified(getUri(), getWorkingDirectory());
-    return dfs.addPathBasedCacheDirective(new PathBasedCacheDirective.Builder().
-        setPath(path).
-        setReplication(directive.getReplication()).
-        setPool(directive.getPool()).
-        build());
+    return dfs.addPathBasedCacheDirective(
+        new PathBasedCacheDirective.Builder(directive).
+            setPath(path).
+            build());
   }
   
+  public void modifyPathBasedCacheDirective(
+      PathBasedCacheDirective directive) throws IOException {
+    if (directive.getPath() != null) {
+      directive = new PathBasedCacheDirective.Builder(directive).
+          setPath(new Path(getPathName(fixRelativePart(directive.getPath()))).
+              makeQualified(getUri(), getWorkingDirectory())).build();
+    }
+    dfs.modifyPathBasedCacheDirective(directive);
+  }
+
   /**
-   * Remove a PathBasedCacheDescriptor.
+   * Remove a PathBasedCacheDirective.
    * 
-   * @param descriptor PathBasedCacheDescriptor to remove
-   * @throws IOException if the descriptor could not be removed
+   * @param id identifier of the PathBasedCacheDirective to remove
+   * @throws IOException if the directive could not be removed
    */
-  public void removePathBasedCacheDescriptor(PathBasedCacheDescriptor descriptor)
+  public void removePathBasedCacheDirective(long id)
       throws IOException {
-    dfs.removePathBasedCacheDescriptor(descriptor.getEntryId());
+    dfs.removePathBasedCacheDirective(id);
   }
   
   /**
    * List the set of cached paths of a cache pool. Incrementally fetches results
    * from the server.
    * 
-   * @param pool The cache pool to list, or null to list all pools.
-   * @param path The path name to list, or null to list all paths.
-   * @return A RemoteIterator which returns PathBasedCacheDescriptor objects.
-   */
-  public RemoteIterator<PathBasedCacheDescriptor> listPathBasedCacheDescriptors(
-      String pool, final Path path) throws IOException {
-    String pathName = path != null ? getPathName(fixRelativePart(path)) : null;
-    final RemoteIterator<PathBasedCacheDescriptor> iter =
-        dfs.listPathBasedCacheDescriptors(pool, pathName);
-    return new RemoteIterator<PathBasedCacheDescriptor>() {
+   * @param filter Filter parameters to use when listing the directives, null to
+   *               list all directives visible to us.
+   * @return A RemoteIterator which returns PathBasedCacheDirective objects.
+   */
+  public RemoteIterator<PathBasedCacheDirective> listPathBasedCacheDirectives(
+      PathBasedCacheDirective filter) throws IOException {
+    if (filter == null) {
+      filter = new PathBasedCacheDirective.Builder().build();
+    }
+    if (filter.getPath() != null) {
+      filter = new PathBasedCacheDirective.Builder(filter).
+          setPath(filter.getPath().
+              makeQualified(getUri(), filter.getPath())).
+                build();
+    }
+    final RemoteIterator<PathBasedCacheDirective> iter =
+        dfs.listPathBasedCacheDirectives(filter);
+    return new RemoteIterator<PathBasedCacheDirective>() {
       @Override
       public boolean hasNext() throws IOException {
         return iter.hasNext();
       }
 
       @Override
-      public PathBasedCacheDescriptor next() throws IOException {
-        PathBasedCacheDescriptor desc = iter.next();
-        Path qualPath = desc.getPath().makeQualified(getUri(), path);
-        return new PathBasedCacheDescriptor(desc.getEntryId(), qualPath,
-            desc.getReplication(), desc.getPool());
+      public PathBasedCacheDirective next() throws IOException {
+        PathBasedCacheDirective desc = iter.next();
+        Path p = desc.getPath().makeQualified(getUri(), desc.getPath());
+        return new PathBasedCacheDirective.Builder(desc).setPath(p).build();
       }
     };
   }

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java Thu Nov  7 22:07:16 2013
@@ -1099,35 +1099,46 @@ public interface ClientProtocol {
    * Add a PathBasedCache entry to the CacheManager.
    * 
    * @param directive A PathBasedCacheDirective to be added
-   * @return A PathBasedCacheDescriptor associated with the added directive
+   * @return A PathBasedCacheDirective associated with the added directive
    * @throws IOException if the directive could not be added
    */
   @AtMostOnce
-  public PathBasedCacheDescriptor addPathBasedCacheDirective(
+  public long addPathBasedCacheDirective(
       PathBasedCacheDirective directive) throws IOException;
 
   /**
-   * Remove a PathBasedCacheDescriptor from the CacheManager.
+   * Modify a PathBasedCache entry in the CacheManager.
    * 
-   * @param id of a PathBasedCacheDescriptor
-   * @throws IOException if the cache descriptor could not be removed
+   * @return directive The directive to modify.  Must contain 
+   *                   a directive ID.
+   * @throws IOException if the directive could not be modified
    */
   @AtMostOnce
-  public void removePathBasedCacheDescriptor(Long id) throws IOException;
+  public void modifyPathBasedCacheDirective(
+      PathBasedCacheDirective directive) throws IOException;
+
+  /**
+   * Remove a PathBasedCacheDirective from the CacheManager.
+   * 
+   * @param id of a PathBasedCacheDirective
+   * @throws IOException if the cache directive could not be removed
+   */
+  @AtMostOnce
+  public void removePathBasedCacheDirective(long id) throws IOException;
 
   /**
    * List the set of cached paths of a cache pool. Incrementally fetches results
    * from the server.
    * 
    * @param prevId The last listed entry ID, or -1 if this is the first call to
-   *          listPathBasedCacheDescriptors.
-   * @param pool The cache pool to list, or null to list all pools.
-   * @param path The path name to list, or null to list all paths.
-   * @return A RemoteIterator which returns PathBasedCacheDescriptor objects.
+   *               listPathBasedCacheDirectives.
+   * @param filter Parameters to use to filter the list results, 
+   *               or null to display all directives visible to us.
+   * @return A RemoteIterator which returns PathBasedCacheDirective objects.
    */
   @Idempotent
-  public RemoteIterator<PathBasedCacheDescriptor> listPathBasedCacheDescriptors(
-      long prevId, String pool, String path) throws IOException;
+  public RemoteIterator<PathBasedCacheDirective> listPathBasedCacheDirectives(
+      long prevId, PathBasedCacheDirective filter) throws IOException;
 
   /**
    * Add a new cache pool.

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheDirective.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheDirective.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheDirective.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheDirective.java Thu Nov  7 22:07:16 2013
@@ -17,32 +17,27 @@
  */
 package org.apache.hadoop.hdfs.protocol;
 
-import java.io.IOException;
-
-import com.google.common.base.Preconditions;
+import java.net.URI;
 
 import org.apache.commons.lang.builder.EqualsBuilder;
 import org.apache.commons.lang.builder.HashCodeBuilder;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.hdfs.DFSUtil;
-import org.apache.hadoop.hdfs.protocol.AddPathBasedCacheDirectiveException.InvalidPoolNameError;
-import org.apache.hadoop.hdfs.protocol.AddPathBasedCacheDirectiveException.InvalidPathNameError;
 
 /**
- * A directive to add a path to a cache pool.
+ * Describes a path-based cache directive.
  */
 @InterfaceStability.Evolving
 @InterfaceAudience.Public
 public class PathBasedCacheDirective {
-
   /**
    * A builder for creating new PathBasedCacheDirective instances.
    */
   public static class Builder {
+    private Long id;
     private Path path;
-    private short replication = (short)1;
+    private Short replication;
     private String pool;
 
     /**
@@ -51,7 +46,37 @@ public class PathBasedCacheDirective {
      * @return New PathBasedCacheDirective.
      */
     public PathBasedCacheDirective build() {
-      return new PathBasedCacheDirective(path, replication, pool);
+      return new PathBasedCacheDirective(id, path, replication, pool);
+    }
+
+    /**
+     * Creates an empty builder.
+     */
+    public Builder() {
+    }
+
+    /**
+     * Creates a builder with all elements set to the same values as the
+     * given PathBasedCacheDirective.
+     */
+    public Builder(PathBasedCacheDirective directive) {
+      this.id = directive.getId();
+      // deep-copy URI
+      URI uri = directive.getPath().toUri();
+      this.path = new Path(uri.getScheme(), uri.getAuthority(), uri.getPath());
+      this.replication = directive.getReplication();
+      this.pool = directive.getPool();
+    }
+
+    /**
+     * Sets the id used in this request.
+     * 
+     * @param id The id used in this request.
+     * @return This builder, for call chaining.
+     */
+    public Builder setId(Long id) {
+      this.id = id;
+      return this;
     }
 
     /**
@@ -71,7 +96,7 @@ public class PathBasedCacheDirective {
      * @param replication The replication used in this request.
      * @return This builder, for call chaining.
      */
-    public Builder setReplication(short replication) {
+    public Builder setReplication(Short replication) {
       this.replication = replication;
       return this;
     }
@@ -88,10 +113,25 @@ public class PathBasedCacheDirective {
     }
   }
 
+  private final Long id;
   private final Path path;
-  private final short replication;
+  private final Short replication;
   private final String pool;
 
+  PathBasedCacheDirective(Long id, Path path, Short replication, String pool) {
+    this.id = id;
+    this.path = path;
+    this.replication = replication;
+    this.pool = pool;
+  }
+
+  /**
+   * @return The ID of this directive.
+   */
+  public Long getId() {
+    return id;
+  }
+
   /**
    * @return The path used in this request.
    */
@@ -102,7 +142,7 @@ public class PathBasedCacheDirective {
   /**
    * @return The number of times the block should be cached.
    */
-  public short getReplication() {
+  public Short getReplication() {
     return replication;
   }
 
@@ -113,25 +153,6 @@ public class PathBasedCacheDirective {
     return pool;
   }
 
-  /**
-   * Check if this PathBasedCacheDirective is valid.
-   * 
-   * @throws IOException
-   *     If this PathBasedCacheDirective is not valid.
-   */
-  public void validate() throws IOException {
-    if (!DFSUtil.isValidName(path.toUri().getPath())) {
-      throw new InvalidPathNameError(this);
-    }
-    if (replication <= 0) {
-      throw new IOException("Tried to request a cache replication " +
-          "factor of " + replication + ", but that is less than 1.");
-    }
-    if (pool.isEmpty()) {
-      throw new InvalidPoolNameError(this);
-    }
-  }
-
   @Override
   public boolean equals(Object o) {
     if (o == null) {
@@ -141,7 +162,8 @@ public class PathBasedCacheDirective {
       return false;
     }
     PathBasedCacheDirective other = (PathBasedCacheDirective)o;
-    return new EqualsBuilder().append(getPath(), other.getPath()).
+    return new EqualsBuilder().append(getId(), other.getId()).
+        append(getPath(), other.getPath()).
         append(getReplication(), other.getReplication()).
         append(getPool(), other.getPool()).
         isEquals();
@@ -149,34 +171,35 @@ public class PathBasedCacheDirective {
 
   @Override
   public int hashCode() {
-    return new HashCodeBuilder().append(getPath()).
+    return new HashCodeBuilder().append(id).
+        append(path).
         append(replication).
-        append(getPool()).
+        append(pool).
         hashCode();
   }
 
   @Override
   public String toString() {
     StringBuilder builder = new StringBuilder();
-    builder.append("{ path:").append(path).
-      append(", replication:").append(replication).
-      append(", pool:").append(pool).
-      append(" }");
+    builder.append("{");
+    String prefix = "";
+    if (id != null) {
+      builder.append(prefix).append("id: ").append(id);
+      prefix = ",";
+    }
+    if (path != null) {
+      builder.append(prefix).append("path: ").append(path);
+      prefix = ",";
+    }
+    if (replication != null) {
+      builder.append(prefix).append("replication: ").append(replication);
+      prefix = ",";
+    }
+    if (pool != null) {
+      builder.append(prefix).append("pool: ").append(pool);
+      prefix = ",";
+    }
+    builder.append("}");
     return builder.toString();
   }
-
-  /**
-   * Protected constructor.  Callers use Builder to create new instances.
-   * 
-   * @param path The path used in this request.
-   * @param replication The replication used in this request.
-   * @param pool The pool used in this request.
-   */
-  protected PathBasedCacheDirective(Path path, short replication, String pool) {
-    Preconditions.checkNotNull(path);
-    Preconditions.checkNotNull(pool);
-    this.path = path;
-    this.replication = replication;
-    this.pool = pool;
-  }
 };

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheEntry.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheEntry.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheEntry.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/PathBasedCacheEntry.java Thu Nov  7 22:07:16 2013
@@ -64,6 +64,15 @@ public final class PathBasedCacheEntry {
     return replication;
   }
 
+  public PathBasedCacheDirective toDirective() {
+    return new PathBasedCacheDirective.Builder().
+        setId(entryId).
+        setPath(new Path(path)).
+        setReplication(replication).
+        setPool(pool.getPoolName()).
+        build();
+  }
+  
   @Override
   public String toString() {
     StringBuilder builder = new StringBuilder();
@@ -75,11 +84,6 @@ public final class PathBasedCacheEntry {
     return builder.toString();
   }
 
-  public PathBasedCacheDescriptor getDescriptor() {
-    return new PathBasedCacheDescriptor(entryId, new Path(path), replication,
-        pool.getPoolName());
-  }
-  
   @Override
   public boolean equals(Object o) {
     if (o == null) { return false; }

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolServerSideTranslatorPB.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolServerSideTranslatorPB.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolServerSideTranslatorPB.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolServerSideTranslatorPB.java Thu Nov  7 22:07:16 2013
@@ -28,7 +28,6 @@ import org.apache.hadoop.fs.Options.Rena
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.RemoteIterator;
 import org.apache.hadoop.fs.permission.FsPermission;
-import org.apache.hadoop.hdfs.protocol.AddPathBasedCacheDirectiveException.EmptyPathError;
 import org.apache.hadoop.hdfs.protocol.CachePoolInfo;
 import org.apache.hadoop.hdfs.protocol.ClientProtocol;
 import org.apache.hadoop.hdfs.protocol.CorruptFileBlocks;
@@ -36,11 +35,7 @@ import org.apache.hadoop.hdfs.protocol.D
 import org.apache.hadoop.hdfs.protocol.HdfsFileStatus;
 import org.apache.hadoop.hdfs.protocol.LocatedBlock;
 import org.apache.hadoop.hdfs.protocol.LocatedBlocks;
-import org.apache.hadoop.hdfs.protocol.PathBasedCacheDescriptor;
 import org.apache.hadoop.hdfs.protocol.PathBasedCacheDirective;
-import org.apache.hadoop.hdfs.protocol.RemovePathBasedCacheDescriptorException.InvalidIdException;
-import org.apache.hadoop.hdfs.protocol.RemovePathBasedCacheDescriptorException.NoSuchIdException;
-import org.apache.hadoop.hdfs.protocol.RemovePathBasedCacheDescriptorException.RemovePermissionDeniedException;
 import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport;
 import org.apache.hadoop.hdfs.protocol.SnapshottableDirectoryStatus;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.AbandonBlockRequestProto;
@@ -111,24 +106,25 @@ import org.apache.hadoop.hdfs.protocol.p
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListCachePoolsResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListCorruptFileBlocksRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListCorruptFileBlocksResponseProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDescriptorsElementProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDescriptorsRequestProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDescriptorsResponseProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDirectivesElementProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDirectivesRequestProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDirectivesResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.MetaSaveRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.MetaSaveResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.MkdirsRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.MkdirsResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ModifyCachePoolRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ModifyCachePoolResponseProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.PathBasedCacheDirectiveProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ModifyPathBasedCacheDirectiveRequestProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ModifyPathBasedCacheDirectiveResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RecoverLeaseRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RecoverLeaseResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RefreshNodesRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RefreshNodesResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RemoveCachePoolRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RemoveCachePoolResponseProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RemovePathBasedCacheDescriptorRequestProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RemovePathBasedCacheDescriptorResponseProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RemovePathBasedCacheDirectiveRequestProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RemovePathBasedCacheDirectiveResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.Rename2RequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.Rename2ResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RenameRequestProto;
@@ -177,7 +173,6 @@ import org.apache.hadoop.security.proto.
 import org.apache.hadoop.security.proto.SecurityProtos.RenewDelegationTokenRequestProto;
 import org.apache.hadoop.security.proto.SecurityProtos.RenewDelegationTokenResponseProto;
 import org.apache.hadoop.security.token.Token;
-
 import org.apache.commons.lang.StringUtils;
 
 import com.google.common.primitives.Shorts;
@@ -1039,69 +1034,64 @@ public class ClientNamenodeProtocolServe
       RpcController controller, AddPathBasedCacheDirectiveRequestProto request)
       throws ServiceException {
     try {
-      PathBasedCacheDirectiveProto proto = request.getDirective();
-      if (StringUtils.isEmpty(proto.getPath())) {
-        throw new EmptyPathError();
-      }
-      PathBasedCacheDirective directive = new PathBasedCacheDirective.Builder().
-          setPath(new Path(proto.getPath())).
-          setReplication(Shorts.checkedCast(proto.getReplication())).
-          setPool(proto.getPool()).
-          build();
-      PathBasedCacheDescriptor descriptor =
-          server.addPathBasedCacheDirective(directive);
-      AddPathBasedCacheDirectiveResponseProto.Builder builder =
-         AddPathBasedCacheDirectiveResponseProto.newBuilder();
-      builder.setDescriptorId(descriptor.getEntryId());
-      return builder.build();
+      return AddPathBasedCacheDirectiveResponseProto.newBuilder().
+              setId(server.addPathBasedCacheDirective(
+                  PBHelper.convert(request.getInfo()))).build();
     } catch (IOException e) {
       throw new ServiceException(e);
     }
   }
 
   @Override
-  public RemovePathBasedCacheDescriptorResponseProto removePathBasedCacheDescriptor(
-      RpcController controller,
-      RemovePathBasedCacheDescriptorRequestProto request)
+  public ModifyPathBasedCacheDirectiveResponseProto modifyPathBasedCacheDirective(
+      RpcController controller, ModifyPathBasedCacheDirectiveRequestProto request)
       throws ServiceException {
     try {
-      server.removePathBasedCacheDescriptor(request.getDescriptorId());
-      RemovePathBasedCacheDescriptorResponseProto.Builder builder =
-         RemovePathBasedCacheDescriptorResponseProto.newBuilder();
-      return builder.build();
+      server.modifyPathBasedCacheDirective(
+          PBHelper.convert(request.getInfo()));
+      return ModifyPathBasedCacheDirectiveResponseProto.newBuilder().build();
+    } catch (IOException e) {
+      throw new ServiceException(e);
+    }
+  }
+
+  @Override
+  public RemovePathBasedCacheDirectiveResponseProto
+      removePathBasedCacheDirective(RpcController controller,
+          RemovePathBasedCacheDirectiveRequestProto request)
+              throws ServiceException {
+    try {
+      server.removePathBasedCacheDirective(request.getId());
+      return RemovePathBasedCacheDirectiveResponseProto.
+          newBuilder().build();
     } catch (IOException e) {
       throw new ServiceException(e);
     }
   }
 
   @Override
-  public ListPathBasedCacheDescriptorsResponseProto listPathBasedCacheDescriptors(
-      RpcController controller, ListPathBasedCacheDescriptorsRequestProto request)
+  public ListPathBasedCacheDirectivesResponseProto listPathBasedCacheDirectives(
+      RpcController controller, ListPathBasedCacheDirectivesRequestProto request)
           throws ServiceException {
     try {
-      RemoteIterator<PathBasedCacheDescriptor> iter =
-         server.listPathBasedCacheDescriptors(request.getPrevId(),
-             request.hasPool() ? request.getPool() : null,
-             request.hasPath() ? request.getPath() : null);
-      ListPathBasedCacheDescriptorsResponseProto.Builder builder =
-          ListPathBasedCacheDescriptorsResponseProto.newBuilder();
+      PathBasedCacheDirective filter =
+          PBHelper.convert(request.getFilter());
+      RemoteIterator<PathBasedCacheDirective> iter =
+         server.listPathBasedCacheDirectives(request.getPrevId(), filter);
+      ListPathBasedCacheDirectivesResponseProto.Builder builder =
+          ListPathBasedCacheDirectivesResponseProto.newBuilder();
       long prevId = 0;
       while (iter.hasNext()) {
-        PathBasedCacheDescriptor directive = iter.next();
+        PathBasedCacheDirective directive = iter.next();
         builder.addElements(
-            ListPathBasedCacheDescriptorsElementProto.newBuilder().
-              setId(directive.getEntryId()).
-              setPath(directive.getPath().toUri().getPath()).
-              setReplication(directive.getReplication()).
-              setPool(directive.getPool()));
-        prevId = directive.getEntryId();
+            ListPathBasedCacheDirectivesElementProto.newBuilder().
+                setInfo(PBHelper.convert(directive)));
+        prevId = directive.getId();
       }
       if (prevId == 0) {
         builder.setHasMore(false);
       } else {
-        iter = server.listPathBasedCacheDescriptors(prevId, 
-            request.hasPool() ? request.getPool() : null,
-            request.hasPath() ? request.getPath() : null);
+        iter = server.listPathBasedCacheDirectives(prevId, filter);
         builder.setHasMore(iter.hasNext());
       }
       return builder.build();

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolTranslatorPB.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolTranslatorPB.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolTranslatorPB.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolTranslatorPB.java Thu Nov  7 22:07:16 2013
@@ -51,7 +51,6 @@ import org.apache.hadoop.hdfs.protocol.H
 import org.apache.hadoop.hdfs.protocol.LocatedBlock;
 import org.apache.hadoop.hdfs.protocol.LocatedBlocks;
 import org.apache.hadoop.hdfs.protocol.NSQuotaExceededException;
-import org.apache.hadoop.hdfs.protocol.PathBasedCacheDescriptor;
 import org.apache.hadoop.hdfs.protocol.PathBasedCacheDirective;
 import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport;
 import org.apache.hadoop.hdfs.protocol.SnapshottableDirectoryStatus;
@@ -101,17 +100,16 @@ import org.apache.hadoop.hdfs.protocol.p
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListCachePoolsResponseElementProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListCachePoolsResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListCorruptFileBlocksRequestProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDescriptorsElementProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDescriptorsRequestProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDescriptorsResponseProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDirectivesRequestProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ListPathBasedCacheDirectivesResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.MetaSaveRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.MkdirsRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ModifyCachePoolRequestProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.PathBasedCacheDirectiveProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ModifyPathBasedCacheDirectiveRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RecoverLeaseRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RefreshNodesRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RemoveCachePoolRequestProto;
-import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RemovePathBasedCacheDescriptorRequestProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RemovePathBasedCacheDirectiveRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.Rename2RequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RenameRequestProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.RenameSnapshotRequestProto;
@@ -1005,55 +1003,53 @@ public class ClientNamenodeProtocolTrans
   }
 
   @Override
-  public PathBasedCacheDescriptor addPathBasedCacheDirective(
+  public long addPathBasedCacheDirective(
       PathBasedCacheDirective directive) throws IOException {
     try {
-      AddPathBasedCacheDirectiveRequestProto.Builder builder =
-          AddPathBasedCacheDirectiveRequestProto.newBuilder();
-      builder.setDirective(PathBasedCacheDirectiveProto.newBuilder()
-          .setPath(directive.getPath().toUri().getPath())
-          .setReplication(directive.getReplication())
-          .setPool(directive.getPool())
-          .build());
-      AddPathBasedCacheDirectiveResponseProto result = 
-          rpcProxy.addPathBasedCacheDirective(null, builder.build());
-      return new PathBasedCacheDescriptor(result.getDescriptorId(),
-          directive.getPath(), directive.getReplication(),
-          directive.getPool());
+      return rpcProxy.addPathBasedCacheDirective(null, 
+              AddPathBasedCacheDirectiveRequestProto.newBuilder().
+                  setInfo(PBHelper.convert(directive)).build()).getId();
     } catch (ServiceException e) {
       throw ProtobufHelper.getRemoteException(e);
     }
   }
 
   @Override
-  public void removePathBasedCacheDescriptor(Long id)
+  public void modifyPathBasedCacheDirective(
+      PathBasedCacheDirective directive) throws IOException {
+    try {
+      rpcProxy.modifyPathBasedCacheDirective(null,
+          ModifyPathBasedCacheDirectiveRequestProto.newBuilder().
+              setInfo(PBHelper.convert(directive)).build());
+    } catch (ServiceException e) {
+      throw ProtobufHelper.getRemoteException(e);
+    }
+  }
+
+  @Override
+  public void removePathBasedCacheDirective(long id)
       throws IOException {
     try {
-      RemovePathBasedCacheDescriptorRequestProto.Builder builder =
-          RemovePathBasedCacheDescriptorRequestProto.newBuilder();
-      builder.setDescriptorId(id);
-      rpcProxy.removePathBasedCacheDescriptor(null, builder.build());
+      rpcProxy.removePathBasedCacheDirective(null,
+          RemovePathBasedCacheDirectiveRequestProto.newBuilder().
+              setId(id).build());
     } catch (ServiceException e) {
       throw ProtobufHelper.getRemoteException(e);
     }
   }
 
   private static class BatchedPathBasedCacheEntries
-      implements BatchedEntries<PathBasedCacheDescriptor> {
-    private ListPathBasedCacheDescriptorsResponseProto response;
+      implements BatchedEntries<PathBasedCacheDirective> {
+    private ListPathBasedCacheDirectivesResponseProto response;
 
-    BatchedPathBasedCacheEntries(ListPathBasedCacheDescriptorsResponseProto response) {
+    BatchedPathBasedCacheEntries(
+        ListPathBasedCacheDirectivesResponseProto response) {
       this.response = response;
     }
 
     @Override
-    public PathBasedCacheDescriptor get(int i) {
-      ListPathBasedCacheDescriptorsElementProto elementProto =
-        response.getElements(i);
-      return new PathBasedCacheDescriptor(elementProto.getId(),
-          new Path(elementProto.getPath()),
-          Shorts.checkedCast(elementProto.getReplication()),
-          elementProto.getPool());
+    public PathBasedCacheDirective get(int i) {
+      return PBHelper.convert(response.getElements(i).getInfo());
     }
 
     @Override
@@ -1068,31 +1064,25 @@ public class ClientNamenodeProtocolTrans
   }
 
   private class PathBasedCacheEntriesIterator
-      extends BatchedRemoteIterator<Long, PathBasedCacheDescriptor> {
-    private final String pool;
-    private final String path;
+      extends BatchedRemoteIterator<Long, PathBasedCacheDirective> {
+    private final PathBasedCacheDirective filter;
 
-    public PathBasedCacheEntriesIterator(long prevKey, String pool, String path) {
+    public PathBasedCacheEntriesIterator(long prevKey,
+        PathBasedCacheDirective filter) {
       super(prevKey);
-      this.pool = pool;
-      this.path = path;
+      this.filter = filter;
     }
 
     @Override
-    public BatchedEntries<PathBasedCacheDescriptor> makeRequest(
+    public BatchedEntries<PathBasedCacheDirective> makeRequest(
         Long nextKey) throws IOException {
-      ListPathBasedCacheDescriptorsResponseProto response;
+      ListPathBasedCacheDirectivesResponseProto response;
       try {
-        ListPathBasedCacheDescriptorsRequestProto.Builder builder =
-            ListPathBasedCacheDescriptorsRequestProto.newBuilder().setPrevId(nextKey);
-        if (pool != null) {
-          builder.setPool(pool);
-        }
-        if (path != null) {
-          builder.setPath(path);
-        }
-        ListPathBasedCacheDescriptorsRequestProto req = builder.build();
-        response = rpcProxy.listPathBasedCacheDescriptors(null, req);
+        response = rpcProxy.listPathBasedCacheDirectives(null,
+            ListPathBasedCacheDirectivesRequestProto.newBuilder().
+                setPrevId(nextKey).
+                setFilter(PBHelper.convert(filter)).
+                build());
       } catch (ServiceException e) {
         throw ProtobufHelper.getRemoteException(e);
       }
@@ -1100,15 +1090,19 @@ public class ClientNamenodeProtocolTrans
     }
 
     @Override
-    public Long elementToPrevKey(PathBasedCacheDescriptor element) {
-      return element.getEntryId();
+    public Long elementToPrevKey(PathBasedCacheDirective element) {
+      return element.getId();
     }
   }
 
   @Override
-  public RemoteIterator<PathBasedCacheDescriptor> listPathBasedCacheDescriptors(long prevId,
-      String pool, String path) throws IOException {
-    return new PathBasedCacheEntriesIterator(prevId, pool, path);
+  public RemoteIterator<PathBasedCacheDirective>
+      listPathBasedCacheDirectives(long prevId,
+          PathBasedCacheDirective filter) throws IOException {
+    if (filter == null) {
+      filter = new PathBasedCacheDirective.Builder().build();
+    }
+    return new PathBasedCacheEntriesIterator(prevId, filter);
   }
 
   @Override

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java Thu Nov  7 22:07:16 2013
@@ -28,6 +28,7 @@ import java.util.List;
 import org.apache.hadoop.fs.ContentSummary;
 import org.apache.hadoop.fs.CreateFlag;
 import org.apache.hadoop.fs.FsServerDefaults;
+import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.ha.HAServiceProtocol.HAServiceState;
 import org.apache.hadoop.ha.proto.HAServiceProtocolProtos;
@@ -37,6 +38,7 @@ import org.apache.hadoop.hdfs.protocol.C
 import org.apache.hadoop.hdfs.protocol.CorruptFileBlocks;
 import org.apache.hadoop.hdfs.protocol.DatanodeID;
 import org.apache.hadoop.hdfs.protocol.DatanodeInfo;
+import org.apache.hadoop.hdfs.protocol.PathBasedCacheDirective;
 import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport;
 import org.apache.hadoop.hdfs.protocol.DatanodeInfo.AdminStates;
 import org.apache.hadoop.hdfs.protocol.DirectoryListing;
@@ -55,6 +57,7 @@ import org.apache.hadoop.hdfs.protocol.p
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.DatanodeReportTypeProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.GetFsStatsResponseProto;
 import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.SafeModeActionProto;
+import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.PathBasedCacheDirectiveInfoProto;
 import org.apache.hadoop.hdfs.protocol.proto.DatanodeProtocolProtos.BalancerBandwidthCommandProto;
 import org.apache.hadoop.hdfs.protocol.proto.DatanodeProtocolProtos.BlockCommandProto;
 import org.apache.hadoop.hdfs.protocol.proto.DatanodeProtocolProtos.BlockIdCommandProto;
@@ -152,6 +155,7 @@ import org.apache.hadoop.util.DataChecks
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
+import com.google.common.primitives.Shorts;
 import com.google.protobuf.ByteString;
 import com.google.protobuf.CodedInputStream;
 
@@ -1559,6 +1563,45 @@ public class PBHelper {
     return DataChecksum.Type.valueOf(type.getNumber());
   }
 
+  public static PathBasedCacheDirectiveInfoProto convert
+      (PathBasedCacheDirective directive) {
+    PathBasedCacheDirectiveInfoProto.Builder builder = 
+        PathBasedCacheDirectiveInfoProto.newBuilder();
+    if (directive.getId() != null) {
+      builder.setId(directive.getId());
+    }
+    if (directive.getPath() != null) {
+      builder.setPath(directive.getPath().toUri().getPath());
+    }
+    if (directive.getReplication() != null) {
+      builder.setReplication(directive.getReplication());
+    }
+    if (directive.getPool() != null) {
+      builder.setPool(directive.getPool());
+    }
+    return builder.build();
+  }
+
+  public static PathBasedCacheDirective convert
+      (PathBasedCacheDirectiveInfoProto proto) {
+    PathBasedCacheDirective.Builder builder =
+        new PathBasedCacheDirective.Builder();
+    if (proto.hasId()) {
+      builder.setId(proto.getId());
+    }
+    if (proto.hasPath()) {
+      builder.setPath(new Path(proto.getPath()));
+    }
+    if (proto.hasReplication()) {
+      builder.setReplication(Shorts.checkedCast(
+          proto.getReplication()));
+    }
+    if (proto.hasPool()) {
+      builder.setPool(proto.getPool());
+    }
+    return builder.build();
+  }
+  
   public static HdfsProtos.ChecksumTypeProto convert(DataChecksum.Type type) {
     return HdfsProtos.ChecksumTypeProto.valueOf(type.id);
   }

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CacheManager.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CacheManager.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CacheManager.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CacheManager.java Thu Nov  7 22:07:16 2013
@@ -19,14 +19,13 @@ package org.apache.hadoop.hdfs.server.na
 
 import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_LIST_CACHE_POOLS_NUM_RESPONSES;
 import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_LIST_CACHE_POOLS_NUM_RESPONSES_DEFAULT;
-import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_LIST_CACHE_DESCRIPTORS_NUM_RESPONSES;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_LIST_CACHE_DIRECTIVES_NUM_RESPONSES;
 import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_CACHING_ENABLED_KEY;
 import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_CACHING_ENABLED_DEFAULT;
-import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_LIST_CACHE_DESCRIPTORS_NUM_RESPONSES_DEFAULT;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_LIST_CACHE_DIRECTIVES_NUM_RESPONSES_DEFAULT;
 import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_PATH_BASED_CACHE_REFRESH_INTERVAL_MS;
 import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_PATH_BASED_CACHE_REFRESH_INTERVAL_MS_DEFAULT;
 
-import java.io.Closeable;
 import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
@@ -44,33 +43,22 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.IdNotFoundException;
 import org.apache.hadoop.fs.BatchedRemoteIterator.BatchedListEntries;
-import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.FsAction;
 import org.apache.hadoop.hdfs.DFSUtil;
 import org.apache.hadoop.hdfs.protocol.Block;
-import org.apache.hadoop.hdfs.protocol.BlockListAsLongs;
-import org.apache.hadoop.hdfs.protocol.BlockListAsLongs.BlockReportIterator;
 import org.apache.hadoop.hdfs.protocol.CachePoolInfo;
 import org.apache.hadoop.hdfs.protocol.DatanodeID;
 import org.apache.hadoop.hdfs.protocol.LocatedBlock;
 import org.apache.hadoop.hdfs.protocol.PathBasedCacheDirective;
-import org.apache.hadoop.hdfs.protocol.PathBasedCacheDescriptor;
-import org.apache.hadoop.hdfs.protocol.AddPathBasedCacheDirectiveException.InvalidPoolNameError;
-import org.apache.hadoop.hdfs.protocol.AddPathBasedCacheDirectiveException.UnexpectedAddPathBasedCacheDirectiveException;
-import org.apache.hadoop.hdfs.protocol.AddPathBasedCacheDirectiveException.PoolWritePermissionDeniedError;
 import org.apache.hadoop.hdfs.protocol.PathBasedCacheEntry;
-import org.apache.hadoop.hdfs.protocol.RemovePathBasedCacheDescriptorException.InvalidIdException;
-import org.apache.hadoop.hdfs.protocol.RemovePathBasedCacheDescriptorException.NoSuchIdException;
-import org.apache.hadoop.hdfs.protocol.RemovePathBasedCacheDescriptorException.UnexpectedRemovePathBasedCacheDescriptorException;
-import org.apache.hadoop.hdfs.protocol.RemovePathBasedCacheDescriptorException.RemovePermissionDeniedException;
 import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo;
 import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager;
 import org.apache.hadoop.hdfs.server.blockmanagement.CacheReplicationMonitor;
 import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor;
 import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor.CachedBlocksList;
 import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor.CachedBlocksList.Type;
-import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.ReplicaState;
 import org.apache.hadoop.hdfs.server.namenode.metrics.NameNodeMetrics;
 import org.apache.hadoop.hdfs.server.namenode.startupprogress.Phase;
 import org.apache.hadoop.hdfs.server.namenode.startupprogress.StartupProgress;
@@ -78,6 +66,7 @@ import org.apache.hadoop.hdfs.server.nam
 import org.apache.hadoop.hdfs.server.namenode.startupprogress.Step;
 import org.apache.hadoop.hdfs.server.namenode.startupprogress.StepType;
 import org.apache.hadoop.io.Text;
+import org.apache.hadoop.security.AccessControlException;
 import org.apache.hadoop.util.GSet;
 import org.apache.hadoop.util.LightWeightGSet;
 import org.apache.hadoop.util.Time;
@@ -111,7 +100,7 @@ public final class CacheManager {
   /**
    * Cache entries, sorted by ID.
    *
-   * listPathBasedCacheDescriptors relies on the ordering of elements in this map 
+   * listPathBasedCacheDirectives relies on the ordering of elements in this map
    * to track what has already been listed by the client.
    */
   private final TreeMap<Long, PathBasedCacheEntry> entriesById =
@@ -143,7 +132,7 @@ public final class CacheManager {
   /**
    * Maximum number of cache pool directives to list in one operation.
    */
-  private final int maxListCacheDescriptorsResponses;
+  private final int maxListCacheDirectivesNumResponses;
 
   /**
    * Interval between scans in milliseconds.
@@ -191,9 +180,9 @@ public final class CacheManager {
     this.maxListCachePoolsResponses = conf.getInt(
         DFS_NAMENODE_LIST_CACHE_POOLS_NUM_RESPONSES,
         DFS_NAMENODE_LIST_CACHE_POOLS_NUM_RESPONSES_DEFAULT);
-    this.maxListCacheDescriptorsResponses = conf.getInt(
-        DFS_NAMENODE_LIST_CACHE_DESCRIPTORS_NUM_RESPONSES,
-        DFS_NAMENODE_LIST_CACHE_DESCRIPTORS_NUM_RESPONSES_DEFAULT);
+    this.maxListCacheDirectivesNumResponses = conf.getInt(
+        DFS_NAMENODE_LIST_CACHE_DIRECTIVES_NUM_RESPONSES,
+        DFS_NAMENODE_LIST_CACHE_DIRECTIVES_NUM_RESPONSES_DEFAULT);
     scanIntervalMs = conf.getLong(
         DFS_NAMENODE_PATH_BASED_CACHE_REFRESH_INTERVAL_MS,
         DFS_NAMENODE_PATH_BASED_CACHE_REFRESH_INTERVAL_MS_DEFAULT);
@@ -266,132 +255,239 @@ public final class CacheManager {
     return nextEntryId++;
   }
 
-  public PathBasedCacheDescriptor addDirective(
-      PathBasedCacheDirective directive, FSPermissionChecker pc)
-      throws IOException {
-    assert namesystem.hasWriteLock();
-    CachePool pool = cachePools.get(directive.getPool());
-    if (pool == null) {
-      LOG.info("addDirective " + directive + ": pool not found.");
-      throw new InvalidPoolNameError(directive);
-    }
-    if ((pc != null) && (!pc.checkPermission(pool, FsAction.WRITE))) {
-      LOG.info("addDirective " + directive + ": write permission denied.");
-      throw new PoolWritePermissionDeniedError(directive);
-    }
-    try {
-      directive.validate();
-    } catch (IOException ioe) {
-      LOG.info("addDirective " + directive + ": validation failed: "
-          + ioe.getClass().getName() + ": " + ioe.getMessage());
-      throw ioe;
-    }
-    
-    // Add a new entry with the next available ID.
-    PathBasedCacheEntry entry;
-    try {
-      entry = new PathBasedCacheEntry(getNextEntryId(),
-          directive.getPath().toUri().getPath(),
-          directive.getReplication(), pool);
-    } catch (IOException ioe) {
-      throw new UnexpectedAddPathBasedCacheDirectiveException(directive);
-    }
-    LOG.info("addDirective " + directive + ": added cache directive "
-        + directive);
-
-    // Success!
-    // First, add it to the various maps
+  private void addInternal(PathBasedCacheEntry entry) {
     entriesById.put(entry.getEntryId(), entry);
-    String path = directive.getPath().toUri().getPath();
+    String path = entry.getPath();
     List<PathBasedCacheEntry> entryList = entriesByPath.get(path);
     if (entryList == null) {
       entryList = new ArrayList<PathBasedCacheEntry>(1);
       entriesByPath.put(path, entryList);
     }
     entryList.add(entry);
+  }
+
+  public PathBasedCacheDirective addDirective(
+      PathBasedCacheDirective directive, FSPermissionChecker pc)
+      throws IOException {
+    assert namesystem.hasWriteLock();
+    PathBasedCacheEntry entry;
+    try {
+      if (directive.getPool() == null) {
+        throw new IdNotFoundException("addDirective: no pool was specified.");
+      }
+      if (directive.getPool().isEmpty()) {
+        throw new IdNotFoundException("addDirective: pool name was empty.");
+      }
+      CachePool pool = cachePools.get(directive.getPool());
+      if (pool == null) {
+        throw new IdNotFoundException("addDirective: no such pool as " +
+            directive.getPool());
+      }
+      if ((pc != null) && (!pc.checkPermission(pool, FsAction.WRITE))) {
+        throw new AccessControlException("addDirective: write " +
+            "permission denied for pool " + directive.getPool());
+      }
+      if (directive.getPath() == null) {
+        throw new IOException("addDirective: no path was specified.");
+      }
+      String path = directive.getPath().toUri().getPath();
+      if (!DFSUtil.isValidName(path)) {
+        throw new IOException("addDirective: path '" + path + "' is invalid.");
+      }
+      short replication = directive.getReplication() == null ? 
+          (short)1 : directive.getReplication();
+      if (replication <= 0) {
+        throw new IOException("addDirective: replication " + replication +
+            " is invalid.");
+      }
+      long id;
+      if (directive.getId() != null) {
+        // We are loading an entry from the edit log.
+        // Use the ID from the edit log.
+        id = directive.getId();
+      } else {
+        // Add a new entry with the next available ID.
+        id = getNextEntryId();
+      }
+      entry = new PathBasedCacheEntry(id, path, replication, pool);
+      addInternal(entry);
+    } catch (IOException e) {
+      LOG.warn("addDirective " + directive + ": failed.", e);
+      throw e;
+    }
+    LOG.info("addDirective " + directive + ": succeeded.");
     if (monitor != null) {
       monitor.kick();
     }
-    return entry.getDescriptor();
+    return entry.toDirective();
   }
 
-  public void removeDescriptor(long id, FSPermissionChecker pc)
-      throws IOException {
+  public void modifyDirective(PathBasedCacheDirective directive,
+      FSPermissionChecker pc) throws IOException {
     assert namesystem.hasWriteLock();
-    // Check for invalid IDs.
-    if (id <= 0) {
-      LOG.info("removeDescriptor " + id + ": invalid non-positive " +
-          "descriptor ID.");
-      throw new InvalidIdException(id);
-    }
-    // Find the entry.
-    PathBasedCacheEntry existing = entriesById.get(id);
-    if (existing == null) {
-      LOG.info("removeDescriptor " + id + ": entry not found.");
-      throw new NoSuchIdException(id);
-    }
-    CachePool pool = cachePools.get(existing.getDescriptor().getPool());
-    if (pool == null) {
-      LOG.info("removeDescriptor " + id + ": pool not found for directive " +
-        existing.getDescriptor());
-      throw new UnexpectedRemovePathBasedCacheDescriptorException(id);
-    }
-    if ((pc != null) && (!pc.checkPermission(pool, FsAction.WRITE))) {
-      LOG.info("removeDescriptor " + id + ": write permission denied to " +
-          "pool " + pool + " for entry " + existing);
-      throw new RemovePermissionDeniedException(id);
+    String idString =
+        (directive.getId() == null) ?
+            "(null)" : directive.getId().toString();
+    try {
+      // Check for invalid IDs.
+      Long id = directive.getId();
+      if (id == null) {
+        throw new IdNotFoundException("modifyDirective: " +
+            "no ID to modify was supplied.");
+      }
+      if (id <= 0) {
+        throw new IdNotFoundException("modifyDirective " + id +
+            ": invalid non-positive directive ID.");
+      }
+      // Find the entry.
+      PathBasedCacheEntry prevEntry = entriesById.get(id);
+      if (prevEntry == null) {
+        throw new IdNotFoundException("modifyDirective " + id +
+            ": id not found.");
+      }
+      if ((pc != null) &&
+          (!pc.checkPermission(prevEntry.getPool(), FsAction.WRITE))) {
+        throw new AccessControlException("modifyDirective " + id +
+            ": permission denied for initial pool " + prevEntry.getPool());
+      }
+      String path = prevEntry.getPath();
+      if (directive.getPath() != null) {
+        path = directive.getPath().toUri().getPath();
+        if (!DFSUtil.isValidName(path)) {
+          throw new IOException("modifyDirective " + id + ": new path " +
+              path + " is not valid.");
+        }
+      }
+      short replication = (directive.getReplication() != null) ?
+          directive.getReplication() : prevEntry.getReplication();
+      if (replication <= 0) {
+        throw new IOException("modifyDirective: replication " + replication +
+            " is invalid.");
+      }
+      CachePool pool = prevEntry.getPool();
+      if (directive.getPool() != null) {
+        pool = cachePools.get(directive.getPool());
+        if (pool == null) {
+          throw new IdNotFoundException("modifyDirective " + id +
+              ": pool " + directive.getPool() + " not found.");
+        }
+        if (directive.getPool().isEmpty()) {
+          throw new IdNotFoundException("modifyDirective: pool name was " +
+              "empty.");
+        }
+        if ((pc != null) &&
+            (!pc.checkPermission(pool, FsAction.WRITE))) {
+          throw new AccessControlException("modifyDirective " + id +
+              ": permission denied for target pool " + pool);
+        }
+      }
+      removeInternal(prevEntry);
+      PathBasedCacheEntry newEntry =
+          new PathBasedCacheEntry(id, path, replication, pool);
+      addInternal(newEntry);
+    } catch (IOException e) {
+      LOG.warn("modifyDirective " + idString + ": failed.", e);
+      throw e;
     }
-    
+    LOG.info("modifyDirective " + idString + ": successfully applied " +
+        directive);
+  }
+
+  public void removeInternal(PathBasedCacheEntry existing)
+      throws IOException {
+    assert namesystem.hasWriteLock();
     // Remove the corresponding entry in entriesByPath.
-    String path = existing.getDescriptor().getPath().toUri().getPath();
+    String path = existing.getPath();
     List<PathBasedCacheEntry> entries = entriesByPath.get(path);
     if (entries == null || !entries.remove(existing)) {
-      throw new UnexpectedRemovePathBasedCacheDescriptorException(id);
+      throw new IdNotFoundException("removeInternal: failed to locate entry " +
+          existing.getEntryId() + " by path " + existing.getPath());
     }
     if (entries.size() == 0) {
       entriesByPath.remove(path);
     }
-    entriesById.remove(id);
+    entriesById.remove(existing.getEntryId());
+  }
+
+  public void removeDirective(long id, FSPermissionChecker pc)
+      throws IOException {
+    assert namesystem.hasWriteLock();
+    try {
+      // Check for invalid IDs.
+      if (id <= 0) {
+        throw new IdNotFoundException("removeDirective " + id + ": invalid " +
+            "non-positive directive ID.");
+      }
+      // Find the entry.
+      PathBasedCacheEntry existing = entriesById.get(id);
+      if (existing == null) {
+        throw new IdNotFoundException("removeDirective " + id +
+            ": id not found.");
+      }
+      if ((pc != null) &&
+          (!pc.checkPermission(existing.getPool(), FsAction.WRITE))) {
+        throw new AccessControlException("removeDirective " + id +
+            ": write permission denied on pool " +
+            existing.getPool().getPoolName());
+      }
+      removeInternal(existing);
+    } catch (IOException e) {
+      LOG.warn("removeDirective " + id + " failed.", e);
+      throw e;
+    }
     if (monitor != null) {
       monitor.kick();
     }
-    LOG.info("removeDescriptor successful for PathCacheEntry id " + id);
+    LOG.info("removeDirective " + id + ": succeeded.");
   }
 
-  public BatchedListEntries<PathBasedCacheDescriptor> 
-        listPathBasedCacheDescriptors(long prevId, String filterPool,
-            String filterPath, FSPermissionChecker pc) throws IOException {
+  public BatchedListEntries<PathBasedCacheDirective> 
+        listPathBasedCacheDirectives(long prevId,
+            PathBasedCacheDirective filter,
+            FSPermissionChecker pc) throws IOException {
     assert namesystem.hasReadOrWriteLock();
     final int NUM_PRE_ALLOCATED_ENTRIES = 16;
-    if (filterPath != null) {
+    String filterPath = null;
+    if (filter.getId() != null) {
+      throw new IOException("we currently don't support filtering by ID");
+    }
+    if (filter.getPath() != null) {
+      filterPath = filter.getPath().toUri().getPath();
       if (!DFSUtil.isValidName(filterPath)) {
-        throw new IOException("invalid path name '" + filterPath + "'");
+        throw new IOException("listPathBasedCacheDirectives: invalid " +
+            "path name '" + filterPath + "'");
       }
     }
-    ArrayList<PathBasedCacheDescriptor> replies =
-        new ArrayList<PathBasedCacheDescriptor>(NUM_PRE_ALLOCATED_ENTRIES);
+    if (filter.getReplication() != null) {
+      throw new IOException("we currently don't support filtering " +
+          "by replication");
+    }
+    ArrayList<PathBasedCacheDirective> replies =
+        new ArrayList<PathBasedCacheDirective>(NUM_PRE_ALLOCATED_ENTRIES);
     int numReplies = 0;
-    SortedMap<Long, PathBasedCacheEntry> tailMap = entriesById.tailMap(prevId + 1);
+    SortedMap<Long, PathBasedCacheEntry> tailMap =
+      entriesById.tailMap(prevId + 1);
     for (Entry<Long, PathBasedCacheEntry> cur : tailMap.entrySet()) {
-      if (numReplies >= maxListCacheDescriptorsResponses) {
-        return new BatchedListEntries<PathBasedCacheDescriptor>(replies, true);
+      if (numReplies >= maxListCacheDirectivesNumResponses) {
+        return new BatchedListEntries<PathBasedCacheDirective>(replies, true);
       }
       PathBasedCacheEntry curEntry = cur.getValue();
-      PathBasedCacheDirective directive = cur.getValue().getDescriptor();
-      if (filterPool != null && 
-          !directive.getPool().equals(filterPool)) {
+      PathBasedCacheDirective directive = cur.getValue().toDirective();
+      if (filter.getPool() != null && 
+          !directive.getPool().equals(filter.getPool())) {
         continue;
       }
       if (filterPath != null &&
           !directive.getPath().toUri().getPath().equals(filterPath)) {
         continue;
       }
-      if (pc.checkPermission(curEntry.getPool(), FsAction.READ)) {
-        replies.add(cur.getValue().getDescriptor());
+      if ((pc == null) ||
+          (pc.checkPermission(curEntry.getPool(), FsAction.READ))) {
+        replies.add(cur.getValue().toDirective());
         numReplies++;
       }
     }
-    return new BatchedListEntries<PathBasedCacheDescriptor>(replies, false);
+    return new BatchedListEntries<PathBasedCacheDirective>(replies, false);
   }
 
   /**
@@ -553,7 +649,8 @@ public final class CacheManager {
           blockManager.getDatanodeManager().getDatanode(datanodeID);
       if (datanode == null || !datanode.isAlive) {
         throw new IOException(
-            "processCacheReport from dead or unregistered datanode: " + datanode);
+            "processCacheReport from dead or unregistered datanode: " +
+            datanode);
       }
       processCacheReportImpl(datanode, blockIds);
     } finally {

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java Thu Nov  7 22:07:16 2013
@@ -39,7 +39,6 @@ import org.apache.hadoop.hdfs.DFSConfigK
 import org.apache.hadoop.hdfs.protocol.CachePoolInfo;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants;
 import org.apache.hadoop.hdfs.protocol.PathBasedCacheDirective;
-import org.apache.hadoop.hdfs.protocol.PathBasedCacheEntry;
 import org.apache.hadoop.hdfs.security.token.delegation.DelegationTokenIdentifier;
 import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.NamenodeRole;
 import org.apache.hadoop.hdfs.server.common.Storage.FormatConfirmable;
@@ -47,6 +46,7 @@ import org.apache.hadoop.hdfs.server.com
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.AddCachePoolOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.AddOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.AddPathBasedCacheDirectiveOp;
+import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.ModifyPathBasedCacheDirectiveOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.AllocateBlockIdOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.AllowSnapshotOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.CancelDelegationTokenOp;
@@ -63,7 +63,7 @@ import org.apache.hadoop.hdfs.server.nam
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.OpInstanceCache;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.ReassignLeaseOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RemoveCachePoolOp;
-import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RemovePathBasedCacheDescriptorOp;
+import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RemovePathBasedCacheDirectiveOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RenameOldOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RenameOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RenameSnapshotOp;
@@ -956,18 +956,25 @@ public class FSEditLog implements LogsPu
   
   void logAddPathBasedCacheDirective(PathBasedCacheDirective directive,
       boolean toLogRpcIds) {
-    AddPathBasedCacheDirectiveOp op = AddPathBasedCacheDirectiveOp.getInstance(
-        cache.get())
-        .setPath(directive.getPath().toUri().getPath())
-        .setReplication(directive.getReplication())
-        .setPool(directive.getPool());
+    AddPathBasedCacheDirectiveOp op =
+        AddPathBasedCacheDirectiveOp.getInstance(cache.get())
+            .setDirective(directive);
     logRpcIds(op, toLogRpcIds);
     logEdit(op);
   }
 
-  void logRemovePathBasedCacheDescriptor(Long id, boolean toLogRpcIds) {
-    RemovePathBasedCacheDescriptorOp op =
-        RemovePathBasedCacheDescriptorOp.getInstance(cache.get()).setId(id);
+  void logModifyPathBasedCacheDirective(
+      PathBasedCacheDirective directive, boolean toLogRpcIds) {
+    ModifyPathBasedCacheDirectiveOp op =
+        ModifyPathBasedCacheDirectiveOp.getInstance(
+            cache.get()).setDirective(directive);
+    logRpcIds(op, toLogRpcIds);
+    logEdit(op);
+  }
+
+  void logRemovePathBasedCacheDirective(Long id, boolean toLogRpcIds) {
+    RemovePathBasedCacheDirectiveOp op =
+        RemovePathBasedCacheDirectiveOp.getInstance(cache.get()).setId(id);
     logRpcIds(op, toLogRpcIds);
     logEdit(op);
   }

Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java?rev=1539839&r1=1539838&r2=1539839&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java Thu Nov  7 22:07:16 2013
@@ -37,7 +37,6 @@ import org.apache.hadoop.hdfs.protocol.H
 import org.apache.hadoop.hdfs.protocol.LayoutVersion;
 import org.apache.hadoop.hdfs.protocol.LayoutVersion.Feature;
 import org.apache.hadoop.hdfs.protocol.LocatedBlock;
-import org.apache.hadoop.hdfs.protocol.PathBasedCacheDescriptor;
 import org.apache.hadoop.hdfs.protocol.PathBasedCacheDirective;
 import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo;
 import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfoUnderConstruction;
@@ -58,9 +57,10 @@ import org.apache.hadoop.hdfs.server.nam
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.GetDelegationTokenOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.MkdirOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.ModifyCachePoolOp;
+import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.ModifyPathBasedCacheDirectiveOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.ReassignLeaseOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RemoveCachePoolOp;
-import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RemovePathBasedCacheDescriptorOp;
+import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RemovePathBasedCacheDirectiveOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RenameOldOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RenameOp;
 import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.RenameSnapshotOp;
@@ -642,23 +642,28 @@ public class FSEditLogLoader {
     }
     case OP_ADD_PATH_BASED_CACHE_DIRECTIVE: {
       AddPathBasedCacheDirectiveOp addOp = (AddPathBasedCacheDirectiveOp) op;
-      PathBasedCacheDirective d = new PathBasedCacheDirective.Builder().
-          setPath(new Path(addOp.path)).
-          setReplication(addOp.replication).
-          setPool(addOp.pool).
-          build();
-      PathBasedCacheDescriptor descriptor =
-          fsNamesys.getCacheManager().addDirective(d, null);
-      if (toAddRetryCache) {
-        fsNamesys.addCacheEntryWithPayload(op.rpcClientId, op.rpcCallId,
-            descriptor);
+      PathBasedCacheDirective result = fsNamesys.
+          getCacheManager().addDirective(addOp.directive, null);
+      if (toAddRetryCache) {
+        Long id = result.getId();
+        fsNamesys.addCacheEntryWithPayload(op.rpcClientId, op.rpcCallId, id);
+      }
+      break;
+    }
+    case OP_MODIFY_PATH_BASED_CACHE_DIRECTIVE: {
+      ModifyPathBasedCacheDirectiveOp modifyOp =
+          (ModifyPathBasedCacheDirectiveOp) op;
+      fsNamesys.getCacheManager().modifyDirective(
+          modifyOp.directive, null);
+      if (toAddRetryCache) {
+        fsNamesys.addCacheEntry(op.rpcClientId, op.rpcCallId);
       }
       break;
     }
-    case OP_REMOVE_PATH_BASED_CACHE_DESCRIPTOR: {
-      RemovePathBasedCacheDescriptorOp removeOp =
-          (RemovePathBasedCacheDescriptorOp) op;
-      fsNamesys.getCacheManager().removeDescriptor(removeOp.id, null);
+    case OP_REMOVE_PATH_BASED_CACHE_DIRECTIVE: {
+      RemovePathBasedCacheDirectiveOp removeOp =
+          (RemovePathBasedCacheDirectiveOp) op;
+      fsNamesys.getCacheManager().removeDirective(removeOp.id, null);
       if (toAddRetryCache) {
         fsNamesys.addCacheEntry(op.rpcClientId, op.rpcCallId);
       }



Mime
View raw message