HDFS-12897. getErasureCodingPolicy should handle .snapshot dir better. Contributed by LiXin Ge.

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirErasureCodingOp.java

@@ -307,7 +307,8 @@ private static List<XAttr> removeErasureCodingPolicyXAttr(
    *
    * @param fsn namespace
    * @param src path
-   * @return {@link ErasureCodingPolicy}
+   * @return {@link ErasureCodingPolicy}, or null if no policy has
+   * been set or the policy is REPLICATION
    * @throws IOException
    * @throws FileNotFoundException if the path does not exist.
    * @throws AccessControlException if no read access
@@ -317,17 +318,25 @@ static ErasureCodingPolicy getErasureCodingPolicy(final FSNamesystem fsn,
       throws IOException, AccessControlException {
     assert fsn.hasReadLock();
 
+    if (FSDirectory.isExactReservedName(src)) {
+      return null;
+    }
+
     FSDirectory fsd = fsn.getFSDirectory();
     final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.READ);
     if (fsn.isPermissionEnabled()) {
       fsn.getFSDirectory().checkPathAccess(pc, iip, FsAction.READ);
     }
 
-    if (iip.getLastINode() == null) {
+    ErasureCodingPolicy ecPolicy;
-      throw new FileNotFoundException("Path not found: " + iip.getPath());
+    if (iip.isDotSnapshotDir()) {
+      ecPolicy = null;
+    } else if (iip.getLastINode() == null) {
+      throw new FileNotFoundException("Path not found: " + src);
+    } else {
+      ecPolicy = getErasureCodingPolicyForPath(fsd, iip);
     }
 
-    ErasureCodingPolicy ecPolicy = getErasureCodingPolicyForPath(fsd, iip);
     if (ecPolicy != null && ecPolicy.isReplicationPolicy()) {
       ecPolicy = null;
     }
@@ -409,7 +418,7 @@ private static ErasureCodingPolicy getErasureCodingPolicyForPath(
         if (inode.isSymlink()) {
           return null;
         }
-        final XAttrFeature xaf = inode.getXAttrFeature();
+        final XAttrFeature xaf = inode.getXAttrFeature(iip.getPathSnapshotId());
         if (xaf != null) {
           XAttr xattr = xaf.getXAttr(XATTR_ERASURECODING_POLICY);
           if (xattr != null) {

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestErasureCodingPolicies.java

@@ -306,6 +306,40 @@ public void testGetErasureCodingPolicyWithSystemDefaultECPolicy() throws Excepti
     verifyErasureCodingInfo(src + "/child1", sysDefaultECPolicy);
   }
 
+  @Test
+  public void testErasureCodingPolicyOnReservedDir() throws IOException {
+    final Path reserveDir = new Path("/.reserved");
+    // verify the EC policy is null, not an exception
+    ErasureCodingPolicy policy = fs.getErasureCodingPolicy(reserveDir);
+    assertNull("Got unexpected erasure coding policy", policy);
+
+    // root EC policy before being set is null, verify the reserved raw dir
+    // is treated as root
+    final Path root = new Path("/");
+    final Path rawRoot = new Path("/.reserved/raw");
+    final Path rawRootSlash = new Path("/.reserved/raw/");
+    assertNull("Got unexpected erasure coding policy",
+        fs.getErasureCodingPolicy(root));
+    assertNull("Got unexpected erasure coding policy",
+        fs.getErasureCodingPolicy(rawRoot));
+    assertNull("Got unexpected erasure coding policy",
+        fs.getErasureCodingPolicy(rawRootSlash));
+
+    // verify the EC policy correctness under the reserved raw dir
+    final Path ecDir = new Path("/ec");
+    fs.mkdirs(ecDir);
+    fs.setErasureCodingPolicy(ecDir, ecPolicy.getName());
+
+    ErasureCodingPolicy policyBase = fs.getErasureCodingPolicy(ecDir);
+    assertEquals("Got unexpected erasure coding policy", ecPolicy,
+        policyBase);
+
+    final Path rawRootEc = new Path("/.reserved/raw/ec");
+    ErasureCodingPolicy policyMap = fs.getErasureCodingPolicy(rawRootEc);
+    assertEquals("Got unexpected erasure coding policy", ecPolicy,
+        policyMap);
+  }
+
   @Test
   public void testGetErasureCodingPolicy() throws Exception {
     List<ErasureCodingPolicy> sysECPolicies =

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestErasureCodingPolicyWithSnapshot.java

@@ -29,10 +29,13 @@
 import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.hdfs.protocol.ErasureCodingPolicy;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction;
+import org.apache.hadoop.hdfs.protocol.SystemErasureCodingPolicies;
 import org.apache.hadoop.util.ToolRunner;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.Rule;
+import org.junit.rules.Timeout;
 
 public class TestErasureCodingPolicyWithSnapshot {
   private MiniDFSCluster cluster;
@@ -47,6 +50,9 @@ public ErasureCodingPolicy getEcPolicy() {
     return StripedFileTestUtil.getDefaultECPolicy();
   }
 
+  @Rule
+  public Timeout globalTimeout = new Timeout(120000);
+
   @Before
   public void setupCluster() throws IOException {
     ecPolicy = getEcPolicy();
@@ -71,7 +77,7 @@ public void shutdownCluster() throws IOException {
    * Test correctness of successive snapshot creation and deletion with erasure
    * coding policies. Create snapshot of ecDir's parent directory.
    */
-  @Test(timeout = 120000)
+  @Test
   public void testSnapshotsOnErasureCodingDirsParentDir() throws Exception {
     final int len = 1024;
     final Path ecDirParent = new Path("/parent");
@@ -109,7 +115,7 @@ public void testSnapshotsOnErasureCodingDirsParentDir() throws Exception {
     // Check that older snapshots still have the old ECPolicy settings
     assertEquals("Got unexpected erasure coding policy", ecPolicy,
         fs.getErasureCodingPolicy(snap1ECDir));
-    assertEquals("Got unexpected erasure coding policy", ecPolicy,
+    assertNull("Expected null erasure coding policy",
         fs.getErasureCodingPolicy(snap2ECDir));
 
     // Verify contents of the snapshotted file
@@ -133,7 +139,7 @@ public void testSnapshotsOnErasureCodingDirsParentDir() throws Exception {
   /**
    * Test creation of snapshot on directory has erasure coding policy.
    */
-  @Test(timeout = 120000)
+  @Test
   public void testSnapshotsOnErasureCodingDir() throws Exception {
     final Path ecDir = new Path("/ecdir");
     fs.mkdirs(ecDir);
@@ -148,7 +154,7 @@ public void testSnapshotsOnErasureCodingDir() throws Exception {
   /**
    * Test verify erasure coding policy is present after restarting the NameNode.
    */
-  @Test(timeout = 120000)
+  @Test
   public void testSnapshotsOnErasureCodingDirAfterNNRestart() throws Exception {
     final Path ecDir = new Path("/ecdir");
     fs.mkdirs(ecDir);
@@ -177,7 +183,7 @@ public void testSnapshotsOnErasureCodingDirAfterNNRestart() throws Exception {
   /**
    * Test copy a snapshot will not preserve its erasure coding policy info.
    */
-  @Test(timeout = 120000)
+  @Test
   public void testCopySnapshotWillNotPreserveErasureCodingPolicy()
       throws Exception {
     final int len = 1024;
@@ -228,4 +234,72 @@ public void testFileStatusAcrossNNRestart() throws IOException {
     ContractTestUtils.assertNotErasureCoded(fs, normalFile);
     ContractTestUtils.assertErasureCoded(fs, ecFile);
   }
+
+  @Test
+  public void testErasureCodingPolicyOnDotSnapshotDir() throws IOException {
+    final Path ecDir = new Path("/ecdir");
+    fs.mkdirs(ecDir);
+    fs.allowSnapshot(ecDir);
+
+    // set erasure coding policy and create snapshot
+    fs.setErasureCodingPolicy(ecDir, ecPolicy.getName());
+    final Path snap = fs.createSnapshot(ecDir, "snap1");
+
+    // verify the EC policy correctness
+    ErasureCodingPolicy ecSnap = fs.getErasureCodingPolicy(snap);
+    assertEquals("Got unexpected erasure coding policy", ecPolicy,
+        ecSnap);
+
+    // verify the EC policy is null, not an exception
+    final Path ecDotSnapshotDir = new Path(ecDir, ".snapshot");
+    ErasureCodingPolicy ecSnap1 = fs.getErasureCodingPolicy(ecDotSnapshotDir);
+    assertNull("Got unexpected erasure coding policy", ecSnap1);
+  }
+
+  /**
+   * Test creation of snapshot on directory which changes its
+   * erasure coding policy.
+   */
+  @Test
+  public void testSnapshotsOnErasureCodingDirAfterECPolicyChanges()
+          throws Exception {
+    final Path ecDir = new Path("/ecdir");
+    fs.mkdirs(ecDir);
+    fs.allowSnapshot(ecDir);
+
+    final Path snap1 = fs.createSnapshot(ecDir, "snap1");
+    assertNull("Expected null erasure coding policy",
+        fs.getErasureCodingPolicy(snap1));
+
+    // Set erasure coding policy
+    final ErasureCodingPolicy ec63Policy = SystemErasureCodingPolicies
+        .getByID(SystemErasureCodingPolicies.RS_6_3_POLICY_ID);
+    fs.setErasureCodingPolicy(ecDir, ec63Policy.getName());
+    final Path snap2 = fs.createSnapshot(ecDir, "snap2");
+    assertEquals("Got unexpected erasure coding policy", ec63Policy,
+        fs.getErasureCodingPolicy(snap2));
+
+    // Verify the EC policy correctness after the unset operation
+    fs.unsetErasureCodingPolicy(ecDir);
+    final Path snap3 = fs.createSnapshot(ecDir, "snap3");
+    assertNull("Expected null erasure coding policy",
+        fs.getErasureCodingPolicy(snap3));
+
+    // Change the erasure coding policy and take another snapshot
+    final ErasureCodingPolicy ec32Policy = SystemErasureCodingPolicies
+        .getByID(SystemErasureCodingPolicies.RS_3_2_POLICY_ID);
+    fs.enableErasureCodingPolicy(ec32Policy.getName());
+    fs.setErasureCodingPolicy(ecDir, ec32Policy.getName());
+    final Path snap4 = fs.createSnapshot(ecDir, "snap4");
+    assertEquals("Got unexpected erasure coding policy", ec32Policy,
+        fs.getErasureCodingPolicy(snap4));
+
+    // Check that older snapshot still have the old ECPolicy settings
+    assertNull("Expected null erasure coding policy",
+        fs.getErasureCodingPolicy(snap1));
+    assertEquals("Got unexpected erasure coding policy", ec63Policy,
+        fs.getErasureCodingPolicy(snap2));
+    assertNull("Expected null erasure coding policy",
+        fs.getErasureCodingPolicy(snap3));
+  }
 }