diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index d4db732008..87b02c490d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -435,6 +435,9 @@ Release 2.7.0 - UNRELEASED HDFS-7474. Avoid resolving path in FSPermissionChecker. (jing9) + HDFS-7459. Consolidate cache-related implementation in FSNamesystem into + a single class. (wheat9) + OPTIMIZATIONS HDFS-7454. Reduce memory footprint for AclEntries in NameNode. diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNDNCacheOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNDNCacheOp.java new file mode 100644 index 0000000000..093ee74970 --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNDNCacheOp.java @@ -0,0 +1,124 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdfs.server.namenode; + +import org.apache.hadoop.fs.BatchedRemoteIterator.BatchedListEntries; +import org.apache.hadoop.fs.CacheFlag; +import org.apache.hadoop.hdfs.protocol.CacheDirectiveEntry; +import org.apache.hadoop.hdfs.protocol.CacheDirectiveInfo; +import org.apache.hadoop.hdfs.protocol.CachePoolEntry; +import org.apache.hadoop.hdfs.protocol.CachePoolInfo; +import org.apache.hadoop.security.AccessControlException; + +import java.io.IOException; +import java.util.EnumSet; + +class FSNDNCacheOp { + static CacheDirectiveInfo addCacheDirective( + FSNamesystem fsn, CacheManager cacheManager, + CacheDirectiveInfo directive, EnumSet flags, + boolean logRetryCache) + throws IOException { + + final FSPermissionChecker pc = getFsPermissionChecker(fsn); + + if (directive.getId() != null) { + throw new IOException("addDirective: you cannot specify an ID " + + "for this operation."); + } + CacheDirectiveInfo effectiveDirective = + cacheManager.addDirective(directive, pc, flags); + fsn.getEditLog().logAddCacheDirectiveInfo(effectiveDirective, + logRetryCache); + return effectiveDirective; + } + + static void modifyCacheDirective( + FSNamesystem fsn, CacheManager cacheManager, CacheDirectiveInfo directive, + EnumSet flags, boolean logRetryCache) throws IOException { + final FSPermissionChecker pc = getFsPermissionChecker(fsn); + + cacheManager.modifyDirective(directive, pc, flags); + fsn.getEditLog().logModifyCacheDirectiveInfo(directive, logRetryCache); + } + + static void removeCacheDirective( + FSNamesystem fsn, CacheManager cacheManager, long id, + boolean logRetryCache) + throws IOException { + final FSPermissionChecker pc = getFsPermissionChecker(fsn); + + cacheManager.removeDirective(id, pc); + fsn.getEditLog().logRemoveCacheDirectiveInfo(id, logRetryCache); + } + + static BatchedListEntries listCacheDirectives( + FSNamesystem fsn, CacheManager cacheManager, + long startId, CacheDirectiveInfo filter) throws IOException { + final FSPermissionChecker pc = getFsPermissionChecker(fsn); + return cacheManager.listCacheDirectives(startId, filter, pc); + } + + static CachePoolInfo addCachePool( + FSNamesystem fsn, CacheManager cacheManager, CachePoolInfo req, + boolean logRetryCache) + throws IOException { + final FSPermissionChecker pc = getFsPermissionChecker(fsn); + + if (pc != null) { + pc.checkSuperuserPrivilege(); + } + CachePoolInfo info = cacheManager.addCachePool(req); + fsn.getEditLog().logAddCachePool(info, logRetryCache); + return info; + } + + static void modifyCachePool( + FSNamesystem fsn, CacheManager cacheManager, CachePoolInfo req, + boolean logRetryCache) throws IOException { + final FSPermissionChecker pc = getFsPermissionChecker(fsn); + if (pc != null) { + pc.checkSuperuserPrivilege(); + } + cacheManager.modifyCachePool(req); + fsn.getEditLog().logModifyCachePool(req, logRetryCache); + } + + static void removeCachePool( + FSNamesystem fsn, CacheManager cacheManager, String cachePoolName, + boolean logRetryCache) throws IOException { + final FSPermissionChecker pc = getFsPermissionChecker(fsn); + if (pc != null) { + pc.checkSuperuserPrivilege(); + } + cacheManager.removeCachePool(cachePoolName); + fsn.getEditLog().logRemoveCachePool(cachePoolName, logRetryCache); + } + + static BatchedListEntries listCachePools( + FSNamesystem fsn, CacheManager cacheManager, String prevKey) + throws IOException { + final FSPermissionChecker pc = getFsPermissionChecker(fsn); + return cacheManager.listCachePools(pc, prevKey); + } + + private static FSPermissionChecker getFsPermissionChecker(FSNamesystem fsn) + throws AccessControlException { + return fsn.isPermissionEnabled() ? fsn.getPermissionChecker() : null; + } +} diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 0a558dd3f6..982798fe34 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -1354,7 +1354,11 @@ && shouldRetrySafeMode(this.safeMode)) { } } } - + + boolean isPermissionEnabled() { + return isPermissionEnabled; + } + /** * We already know that the safemode is on. We will throw a RetriableException * if the safemode is not manual or caused by low resource. @@ -3607,7 +3611,7 @@ private boolean deleteInt(String src, boolean recursive, boolean logRetryCache) return status; } - private FSPermissionChecker getPermissionChecker() + FSPermissionChecker getPermissionChecker() throws AccessControlException { return dir.getPermissionChecker(); } @@ -7541,52 +7545,38 @@ long addCacheDirective(CacheDirectiveInfo directive, EnumSet flags, boolean logRetryCache) throws IOException { checkOperation(OperationCategory.WRITE); - final FSPermissionChecker pc = isPermissionEnabled ? - getPermissionChecker() : null; - + CacheDirectiveInfo effectiveDirective = null; if (!flags.contains(CacheFlag.FORCE)) { cacheManager.waitForRescanIfNeeded(); } - boolean success = false; writeLock(); - String effectiveDirectiveStr = null; - Long result = null; try { checkOperation(OperationCategory.WRITE); if (isInSafeMode()) { throw new SafeModeException( "Cannot add cache directive", safeMode); } - if (directive.getId() != null) { - throw new IOException("addDirective: you cannot specify an ID " + - "for this operation."); - } - CacheDirectiveInfo effectiveDirective = - cacheManager.addDirective(directive, pc, flags); - getEditLog().logAddCacheDirectiveInfo(effectiveDirective, logRetryCache); - result = effectiveDirective.getId(); - effectiveDirectiveStr = effectiveDirective.toString(); - success = true; + effectiveDirective = FSNDNCacheOp.addCacheDirective(this, cacheManager, + directive, flags, logRetryCache); } finally { writeUnlock(); + boolean success = effectiveDirective != null; if (success) { getEditLog().logSync(); } - if (isAuditEnabled() && isExternalInvocation()) { - logAuditEvent(success, "addCacheDirective", effectiveDirectiveStr, null, null); - } + String effectiveDirectiveStr = effectiveDirective != null ? + effectiveDirective.toString() : null; + logAuditEvent(success, "addCacheDirective", effectiveDirectiveStr, + null, null); } - return result; + return effectiveDirective != null ? effectiveDirective.getId() : 0; } void modifyCacheDirective(CacheDirectiveInfo directive, EnumSet flags, boolean logRetryCache) throws IOException { checkOperation(OperationCategory.WRITE); - final FSPermissionChecker pc = isPermissionEnabled ? - getPermissionChecker() : null; boolean success = false; - if (!flags.contains(CacheFlag.FORCE)) { cacheManager.waitForRescanIfNeeded(); } @@ -7597,26 +7587,22 @@ void modifyCacheDirective(CacheDirectiveInfo directive, throw new SafeModeException( "Cannot add cache directive", safeMode); } - cacheManager.modifyDirective(directive, pc, flags); - getEditLog().logModifyCacheDirectiveInfo(directive, logRetryCache); + FSNDNCacheOp.modifyCacheDirective(this, cacheManager, directive, flags, + logRetryCache); success = true; } finally { writeUnlock(); if (success) { getEditLog().logSync(); } - if (isAuditEnabled() && isExternalInvocation()) { - String idStr = "{id: " + directive.getId().toString() + "}"; - logAuditEvent(success, "modifyCacheDirective", idStr, directive.toString(), null); - } + String idStr = "{id: " + directive.getId().toString() + "}"; + logAuditEvent(success, "modifyCacheDirective", idStr, + directive.toString(), null); } } void removeCacheDirective(long id, boolean logRetryCache) throws IOException { checkOperation(OperationCategory.WRITE); - final FSPermissionChecker pc = isPermissionEnabled ? - getPermissionChecker() : null; - boolean success = false; writeLock(); try { @@ -7625,16 +7611,13 @@ void removeCacheDirective(long id, boolean logRetryCache) throws IOException { throw new SafeModeException( "Cannot remove cache directives", safeMode); } - cacheManager.removeDirective(id, pc); - getEditLog().logRemoveCacheDirectiveInfo(id, logRetryCache); + FSNDNCacheOp.removeCacheDirective(this, cacheManager, id, logRetryCache); success = true; } finally { writeUnlock(); - if (isAuditEnabled() && isExternalInvocation()) { - String idStr = "{id: " + Long.toString(id) + "}"; - logAuditEvent(success, "removeCacheDirective", idStr, null, - null); - } + String idStr = "{id: " + Long.toString(id) + "}"; + logAuditEvent(success, "removeCacheDirective", idStr, null, + null); } getEditLog().logSync(); } @@ -7642,33 +7625,26 @@ void removeCacheDirective(long id, boolean logRetryCache) throws IOException { BatchedListEntries listCacheDirectives( long startId, CacheDirectiveInfo filter) throws IOException { checkOperation(OperationCategory.READ); - final FSPermissionChecker pc = isPermissionEnabled ? - getPermissionChecker() : null; BatchedListEntries results; cacheManager.waitForRescanIfNeeded(); readLock(); boolean success = false; try { checkOperation(OperationCategory.READ); - results = - cacheManager.listCacheDirectives(startId, filter, pc); + results = FSNDNCacheOp.listCacheDirectives(this, cacheManager, startId, + filter); success = true; } finally { readUnlock(); - if (isAuditEnabled() && isExternalInvocation()) { - logAuditEvent(success, "listCacheDirectives", filter.toString(), null, - null); - } + logAuditEvent(success, "listCacheDirectives", filter.toString(), null, + null); } return results; } - public void addCachePool(CachePoolInfo req, boolean logRetryCache) + void addCachePool(CachePoolInfo req, boolean logRetryCache) throws IOException { checkOperation(OperationCategory.WRITE); - final FSPermissionChecker pc = isPermissionEnabled ? - getPermissionChecker() : null; - writeLock(); boolean success = false; String poolInfoStr = null; @@ -7678,29 +7654,21 @@ public void addCachePool(CachePoolInfo req, boolean logRetryCache) throw new SafeModeException( "Cannot add cache pool " + req.getPoolName(), safeMode); } - if (pc != null) { - pc.checkSuperuserPrivilege(); - } - CachePoolInfo info = cacheManager.addCachePool(req); + CachePoolInfo info = FSNDNCacheOp.addCachePool(this, cacheManager, req, + logRetryCache); poolInfoStr = info.toString(); - getEditLog().logAddCachePool(info, logRetryCache); success = true; } finally { writeUnlock(); - if (isAuditEnabled() && isExternalInvocation()) { - logAuditEvent(success, "addCachePool", poolInfoStr, null, null); - } + logAuditEvent(success, "addCachePool", poolInfoStr, null, null); } getEditLog().logSync(); } - public void modifyCachePool(CachePoolInfo req, boolean logRetryCache) + void modifyCachePool(CachePoolInfo req, boolean logRetryCache) throws IOException { checkOperation(OperationCategory.WRITE); - final FSPermissionChecker pc = - isPermissionEnabled ? getPermissionChecker() : null; - writeLock(); boolean success = false; try { @@ -7709,29 +7677,22 @@ public void modifyCachePool(CachePoolInfo req, boolean logRetryCache) throw new SafeModeException( "Cannot modify cache pool " + req.getPoolName(), safeMode); } - if (pc != null) { - pc.checkSuperuserPrivilege(); - } - cacheManager.modifyCachePool(req); - getEditLog().logModifyCachePool(req, logRetryCache); + FSNDNCacheOp.modifyCachePool(this, cacheManager, req, logRetryCache); success = true; } finally { writeUnlock(); - if (isAuditEnabled() && isExternalInvocation()) { - String poolNameStr = "{poolName: " + req.getPoolName() + "}"; - logAuditEvent(success, "modifyCachePool", poolNameStr, req.toString(), null); - } + String poolNameStr = "{poolName: " + + (req == null ? null : req.getPoolName()) + "}"; + logAuditEvent(success, "modifyCachePool", poolNameStr, + req == null ? null : req.toString(), null); } getEditLog().logSync(); } - public void removeCachePool(String cachePoolName, boolean logRetryCache) + void removeCachePool(String cachePoolName, boolean logRetryCache) throws IOException { checkOperation(OperationCategory.WRITE); - final FSPermissionChecker pc = - isPermissionEnabled ? getPermissionChecker() : null; - writeLock(); boolean success = false; try { @@ -7740,27 +7701,20 @@ public void removeCachePool(String cachePoolName, boolean logRetryCache) throw new SafeModeException( "Cannot remove cache pool " + cachePoolName, safeMode); } - if (pc != null) { - pc.checkSuperuserPrivilege(); - } - cacheManager.removeCachePool(cachePoolName); - getEditLog().logRemoveCachePool(cachePoolName, logRetryCache); + FSNDNCacheOp.removeCachePool(this, cacheManager, cachePoolName, + logRetryCache); success = true; } finally { writeUnlock(); - if (isAuditEnabled() && isExternalInvocation()) { - String poolNameStr = "{poolName: " + cachePoolName + "}"; - logAuditEvent(success, "removeCachePool", poolNameStr, null, null); - } + String poolNameStr = "{poolName: " + cachePoolName + "}"; + logAuditEvent(success, "removeCachePool", poolNameStr, null, null); } getEditLog().logSync(); } - public BatchedListEntries listCachePools(String prevKey) + BatchedListEntries listCachePools(String prevKey) throws IOException { - final FSPermissionChecker pc = - isPermissionEnabled ? getPermissionChecker() : null; BatchedListEntries results; checkOperation(OperationCategory.READ); boolean success = false; @@ -7768,13 +7722,11 @@ public BatchedListEntries listCachePools(String prevKey) readLock(); try { checkOperation(OperationCategory.READ); - results = cacheManager.listCachePools(pc, prevKey); + results = FSNDNCacheOp.listCachePools(this, cacheManager, prevKey); success = true; } finally { readUnlock(); - if (isAuditEnabled() && isExternalInvocation()) { - logAuditEvent(success, "listCachePools", null, null, null); - } + logAuditEvent(success, "listCachePools", null, null, null); } return results; }