diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES-fs-encryption.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES-fs-encryption.txt index 24cfbfb5b20..311e42c6dae 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES-fs-encryption.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES-fs-encryption.txt @@ -52,6 +52,8 @@ fs-encryption (Unreleased) HDFS-6716. Update usage of KeyProviderCryptoExtension APIs on NameNode. (wang) + HDFS-6718. Remove EncryptionZoneManager lock. (wang) + OPTIMIZATIONS BUG FIXES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java index 890200d787b..0f7f61c264d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java @@ -60,35 +60,6 @@ public class EncryptionZoneManager { } - /** - * Protects the encryptionZones map and its contents. - */ - private final ReentrantReadWriteLock lock; - - private void readLock() { - lock.readLock().lock(); - } - - private void readUnlock() { - lock.readLock().unlock(); - } - - private void writeLock() { - lock.writeLock().lock(); - } - - private void writeUnlock() { - lock.writeLock().unlock(); - } - - public boolean hasWriteLock() { - return lock.isWriteLockedByCurrentThread(); - } - - public boolean hasReadLock() { - return lock.getReadHoldCount() > 0 || hasWriteLock(); - } - private final Map encryptionZones; private final FSDirectory dir; private final KeyProvider provider; @@ -101,7 +72,6 @@ public class EncryptionZoneManager { public EncryptionZoneManager(FSDirectory dir, KeyProvider provider) { this.dir = dir; this.provider = provider; - lock = new ReentrantReadWriteLock(); encryptionZones = new HashMap(); } @@ -116,12 +86,7 @@ public class EncryptionZoneManager { void addEncryptionZone(Long inodeId, String keyId) { assert dir.hasWriteLock(); final EncryptionZoneInt ez = new EncryptionZoneInt(inodeId, keyId); - writeLock(); - try { - encryptionZones.put(inodeId, ez); - } finally { - writeUnlock(); - } + encryptionZones.put(inodeId, ez); } /** @@ -131,12 +96,7 @@ public class EncryptionZoneManager { */ void removeEncryptionZone(Long inodeId) { assert dir.hasWriteLock(); - writeLock(); - try { - encryptionZones.remove(inodeId); - } finally { - writeUnlock(); - } + encryptionZones.remove(inodeId); } /** @@ -147,12 +107,7 @@ public class EncryptionZoneManager { boolean isInAnEZ(INodesInPath iip) throws UnresolvedLinkException, SnapshotAccessControlException { assert dir.hasReadLock(); - readLock(); - try { - return (getEncryptionZoneForPath(iip) != null); - } finally { - readUnlock(); - } + return (getEncryptionZoneForPath(iip) != null); } /** @@ -172,16 +127,12 @@ public class EncryptionZoneManager { * Called while holding the FSDirectory lock. */ String getKeyName(final INodesInPath iip) { - readLock(); - try { - EncryptionZoneInt ezi = getEncryptionZoneForPath(iip); - if (ezi == null) { - return null; - } - return ezi.getKeyName(); - } finally { - readUnlock(); + assert dir.hasReadLock(); + EncryptionZoneInt ezi = getEncryptionZoneForPath(iip); + if (ezi == null) { + return null; } + return ezi.getKeyName(); } /** @@ -191,7 +142,7 @@ public class EncryptionZoneManager { * Must be called while holding the manager lock. */ private EncryptionZoneInt getEncryptionZoneForPath(INodesInPath iip) { - assert hasReadLock(); + assert dir.hasReadLock(); Preconditions.checkNotNull(iip); final INode[] inodes = iip.getINodes(); for (int i = inodes.length - 1; i >= 0; i--) { @@ -220,41 +171,36 @@ public class EncryptionZoneManager { void checkMoveValidity(INodesInPath srcIIP, INodesInPath dstIIP, String src) throws IOException { assert dir.hasReadLock(); - readLock(); - try { - final EncryptionZoneInt srcEZI = getEncryptionZoneForPath(srcIIP); - final EncryptionZoneInt dstEZI = getEncryptionZoneForPath(dstIIP); - final boolean srcInEZ = (srcEZI != null); - final boolean dstInEZ = (dstEZI != null); - if (srcInEZ) { - if (!dstInEZ) { - throw new IOException( - src + " can't be moved from an encryption zone."); - } - } else { - if (dstInEZ) { - throw new IOException( - src + " can't be moved into an encryption zone."); - } + final EncryptionZoneInt srcEZI = getEncryptionZoneForPath(srcIIP); + final EncryptionZoneInt dstEZI = getEncryptionZoneForPath(dstIIP); + final boolean srcInEZ = (srcEZI != null); + final boolean dstInEZ = (dstEZI != null); + if (srcInEZ) { + if (!dstInEZ) { + throw new IOException( + src + " can't be moved from an encryption zone."); } + } else { + if (dstInEZ) { + throw new IOException( + src + " can't be moved into an encryption zone."); + } + } - if (srcInEZ || dstInEZ) { - Preconditions.checkState(srcEZI != null, "couldn't find src EZ?"); - Preconditions.checkState(dstEZI != null, "couldn't find dst EZ?"); - if (srcEZI != dstEZI) { - final String srcEZPath = getFullPathName(srcEZI); - final String dstEZPath = getFullPathName(dstEZI); - final StringBuilder sb = new StringBuilder(src); - sb.append(" can't be moved from encryption zone "); - sb.append(srcEZPath); - sb.append(" to encryption zone "); - sb.append(dstEZPath); - sb.append("."); - throw new IOException(sb.toString()); - } + if (srcInEZ || dstInEZ) { + Preconditions.checkState(srcEZI != null, "couldn't find src EZ?"); + Preconditions.checkState(dstEZI != null, "couldn't find dst EZ?"); + if (srcEZI != dstEZI) { + final String srcEZPath = getFullPathName(srcEZI); + final String dstEZPath = getFullPathName(dstEZI); + final StringBuilder sb = new StringBuilder(src); + sb.append(" can't be moved from encryption zone "); + sb.append(srcEZPath); + sb.append(" to encryption zone "); + sb.append(dstEZPath); + sb.append("."); + throw new IOException(sb.toString()); } - } finally { - readUnlock(); } } @@ -266,34 +212,29 @@ public class EncryptionZoneManager { XAttr createEncryptionZone(String src, String keyId, KeyVersion keyVersion) throws IOException { assert dir.hasWriteLock(); - writeLock(); - try { - if (dir.isNonEmptyDirectory(src)) { - throw new IOException( - "Attempt to create an encryption zone for a non-empty directory."); - } - - final INodesInPath srcIIP = dir.getINodesInPath4Write(src, false); - EncryptionZoneInt ezi = getEncryptionZoneForPath(srcIIP); - if (ezi != null) { - throw new IOException("Directory " + src + " is already in an " + - "encryption zone. (" + getFullPathName(ezi) + ")"); - } - - final XAttr keyIdXAttr = XAttrHelper - .buildXAttr(CRYPTO_XATTR_ENCRYPTION_ZONE, keyId.getBytes()); - - final List xattrs = Lists.newArrayListWithCapacity(1); - xattrs.add(keyIdXAttr); - // updating the xattr will call addEncryptionZone, - // done this way to handle edit log loading - dir.unprotectedSetXAttrs(src, xattrs, EnumSet.of(XAttrSetFlag.CREATE)); - // Re-get the new encryption zone add the latest key version - ezi = getEncryptionZoneForPath(srcIIP); - return keyIdXAttr; - } finally { - writeUnlock(); + if (dir.isNonEmptyDirectory(src)) { + throw new IOException( + "Attempt to create an encryption zone for a non-empty directory."); } + + final INodesInPath srcIIP = dir.getINodesInPath4Write(src, false); + EncryptionZoneInt ezi = getEncryptionZoneForPath(srcIIP); + if (ezi != null) { + throw new IOException("Directory " + src + " is already in an " + + "encryption zone. (" + getFullPathName(ezi) + ")"); + } + + final XAttr keyIdXAttr = XAttrHelper + .buildXAttr(CRYPTO_XATTR_ENCRYPTION_ZONE, keyId.getBytes()); + + final List xattrs = Lists.newArrayListWithCapacity(1); + xattrs.add(keyIdXAttr); + // updating the xattr will call addEncryptionZone, + // done this way to handle edit log loading + dir.unprotectedSetXAttrs(src, xattrs, EnumSet.of(XAttrSetFlag.CREATE)); + // Re-get the new encryption zone add the latest key version + ezi = getEncryptionZoneForPath(srcIIP); + return keyIdXAttr; } /** @@ -303,16 +244,11 @@ public class EncryptionZoneManager { */ List listEncryptionZones() throws IOException { assert dir.hasReadLock(); - readLock(); - try { - final List ret = - Lists.newArrayListWithExpectedSize(encryptionZones.size()); - for (EncryptionZoneInt ezi : encryptionZones.values()) { - ret.add(new EncryptionZone(getFullPathName(ezi), ezi.getKeyName())); - } - return ret; - } finally { - readUnlock(); + final List ret = + Lists.newArrayListWithExpectedSize(encryptionZones.size()); + for (EncryptionZoneInt ezi : encryptionZones.values()) { + ret.add(new EncryptionZone(getFullPathName(ezi), ezi.getKeyName())); } + return ret; } }