From 5333e872e2a07de8ff2431c9ff041bb582e8e173 Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Fri, 12 Nov 2021 14:32:02 +0800 Subject: [PATCH] HDFS-16187. SnapshotDiff behaviour with Xattrs and Acls is not consistent across NN restarts with checkpointing (#3340) (#3640) (cherry picked from commit 356ebbbe80aef991d564a6140e341ddd76176416) Conflicts: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java Change-Id: If76fd0d77fafc90fe2f2c19ab1d0c43a58510f6b Co-authored-by: bshashikant --- .../hdfs/server/namenode/XAttrFeature.java | 17 ++++++++++ .../server/namenode/snapshot/Snapshot.java | 15 +++++++++ .../hadoop/hdfs/TestEncryptionZones.java | 31 ++++++++++++++++--- .../snapshot/TestXAttrWithSnapshot.java | 26 ++++++++++++++++ 4 files changed, 85 insertions(+), 4 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrFeature.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrFeature.java index 73df932f2f..11263bb9a0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrFeature.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrFeature.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hdfs.server.namenode; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import org.apache.hadoop.classification.InterfaceAudience; @@ -84,6 +85,22 @@ public List getXAttrs() { } } + @Override + public boolean equals(Object o) { + if (o == null) { + return false; + } + if (getClass() != o.getClass()) { + return false; + } + return getXAttrs().equals(((XAttrFeature) o).getXAttrs()); + } + + @Override + public int hashCode() { + return Arrays.hashCode(getXAttrs().toArray()); + } + /** * Get XAttr by name with prefix. * @param prefixedName xAttr name with prefix diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java index 710acd9791..512c90b494 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/Snapshot.java @@ -24,11 +24,14 @@ import java.util.Arrays; import java.util.Comparator; import java.util.Date; +import java.util.Objects; + import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hdfs.DFSUtil; import org.apache.hadoop.hdfs.protocol.HdfsConstants; import org.apache.hadoop.hdfs.server.namenode.AclFeature; +import org.apache.hadoop.hdfs.server.namenode.INodeDirectoryAttributes; import org.apache.hadoop.hdfs.server.namenode.ContentSummaryComputationContext; import org.apache.hadoop.hdfs.server.namenode.DirectoryWithQuotaFeature; import org.apache.hadoop.hdfs.server.namenode.FSImageFormat; @@ -185,6 +188,18 @@ public ContentSummaryComputationContext computeContentSummary( return computeDirectoryContentSummary(summary, snapshotId); } + @Override + public boolean metadataEquals(INodeDirectoryAttributes other) { + return other != null && getQuotaCounts().equals(other.getQuotaCounts()) + && getPermissionLong() == other.getPermissionLong() + // Acl feature maintains a reference counted map, thereby + // every snapshot copy should point to the same Acl object unless + // there is no change in acl values. + // Reference equals is hence intentional here. + && getAclFeature() == other.getAclFeature() + && Objects.equals(getXAttrFeature(), other.getXAttrFeature()); + } + @Override public String getFullPathName() { return getSnapshotPath(getParent().getFullPathName(), getLocalName()); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java index 2bd2324491..86b90db3aa 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java @@ -43,8 +43,6 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; -import org.apache.hadoop.thirdparty.com.google.common.collect.Lists; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.crypto.CipherSuite; import org.apache.hadoop.crypto.CryptoInputStream; @@ -74,9 +72,10 @@ import org.apache.hadoop.hdfs.client.HdfsAdmin; import org.apache.hadoop.hdfs.protocol.ClientProtocol; import org.apache.hadoop.hdfs.protocol.EncryptionZone; +import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction; import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; import org.apache.hadoop.hdfs.protocol.LocatedBlocks; -import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction; +import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport; import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport.DiffReportEntry; import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport.DiffType; import org.apache.hadoop.hdfs.server.common.HdfsServerConstants; @@ -98,6 +97,7 @@ import org.apache.hadoop.security.authorize.AuthorizationException; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.DelegationTokenIssuer; +import org.apache.hadoop.thirdparty.com.google.common.collect.Lists; import org.apache.hadoop.util.DataChecksum; import org.apache.hadoop.util.ToolRunner; import org.apache.hadoop.crypto.key.KeyProviderDelegationTokenExtension.DelegationTokenExtension; @@ -1184,6 +1184,30 @@ private void dTIEM(Path prefix) throws Exception { } } + @Test + public void testEncryptionZonesWithSnapshots() throws Exception { + final Path snapshottable = new Path("/zones"); + fsWrapper.mkdir(snapshottable, FsPermission.getDirDefault(), + true); + dfsAdmin.allowSnapshot(snapshottable); + dfsAdmin.createEncryptionZone(snapshottable, TEST_KEY, NO_TRASH); + fs.createSnapshot(snapshottable, "snap1"); + SnapshotDiffReport report = + fs.getSnapshotDiffReport(snapshottable, "snap1", ""); + Assert.assertEquals(0, report.getDiffList().size()); + report = + fs.getSnapshotDiffReport(snapshottable, "snap1", ""); + System.out.println(report); + Assert.assertEquals(0, report.getDiffList().size()); + fs.setSafeMode(SafeModeAction.SAFEMODE_ENTER); + fs.saveNamespace(); + fs.setSafeMode(SafeModeAction.SAFEMODE_LEAVE); + cluster.restartNameNode(true); + report = + fs.getSnapshotDiffReport(snapshottable, "snap1", ""); + Assert.assertEquals(0, report.getDiffList().size()); + } + private class AuthorizationExceptionInjector extends EncryptionFaultInjector { @Override public void ensureKeyIsInitialized() throws IOException { @@ -1730,7 +1754,6 @@ public void testEncryptionZonesOnRootPath() throws Exception { true, fs.getFileStatus(rootDir).isEncrypted()); assertEquals("File is encrypted", true, fs.getFileStatus(zoneFile).isEncrypted()); - DFSTestUtil.verifyFilesNotEqual(fs, zoneFile, rawFile, len); } @Test diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestXAttrWithSnapshot.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestXAttrWithSnapshot.java index 2c93e12bc0..badff68212 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestXAttrWithSnapshot.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestXAttrWithSnapshot.java @@ -36,6 +36,7 @@ import org.apache.hadoop.hdfs.protocol.HdfsConstants; import org.apache.hadoop.hdfs.protocol.NSQuotaExceededException; import org.apache.hadoop.hdfs.protocol.SnapshotAccessControlException; +import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport; import org.apache.hadoop.hdfs.server.namenode.NameNode; import org.apache.hadoop.hdfs.server.namenode.NameNodeAdapter; import org.apache.hadoop.io.IOUtils; @@ -140,6 +141,31 @@ public void testModifyReadsCurrentState() throws Exception { assertEquals(xattrs.size(), 0); } + @Test + public void testXattrWithSnapshotAndNNRestart() throws Exception { + // Init + FileSystem.mkdirs(hdfs, path, FsPermission.createImmutable((short) 0700)); + hdfs.setXAttr(path, name1, value1); + hdfs.allowSnapshot(path); + hdfs.createSnapshot(path, snapshotName); + SnapshotDiffReport report = + hdfs.getSnapshotDiffReport(path, snapshotName, ""); + System.out.println(report); + Assert.assertEquals(0, report.getDiffList().size()); + report = + hdfs.getSnapshotDiffReport(path, snapshotName, ""); + System.out.println(report); + Assert.assertEquals(0, report.getDiffList().size()); + hdfs.setSafeMode(HdfsConstants.SafeModeAction.SAFEMODE_ENTER); + hdfs.saveNamespace(); + hdfs.setSafeMode(HdfsConstants.SafeModeAction.SAFEMODE_LEAVE); + cluster.restartNameNode(true); + report = + hdfs.getSnapshotDiffReport(path, snapshotName, ""); + System.out.println(report); + Assert.assertEquals(0, report.getDiffList().size()); + } + /** * Tests removing xattrs on a directory that has been snapshotted */