Revert "HDFS-10762. Pass IIP for file status related methods. Contributed by Daryn Sharp."

This reverts commit 22fc46d765.
This commit is contained in:
Kihwal Lee 2016-08-22 16:57:45 -05:00
parent f4d4d3474c
commit 3ca4d6ddfd
6 changed files with 64 additions and 102 deletions

View File

@ -85,10 +85,9 @@ final class FSDirAppendOp {
final LocatedBlock lb; final LocatedBlock lb;
final FSDirectory fsd = fsn.getFSDirectory(); final FSDirectory fsd = fsn.getFSDirectory();
final String src; final String src;
final INodesInPath iip;
fsd.writeLock(); fsd.writeLock();
try { try {
iip = fsd.resolvePathForWrite(pc, srcArg); final INodesInPath iip = fsd.resolvePathForWrite(pc, srcArg);
src = iip.getPath(); src = iip.getPath();
// Verify that the destination does not exist as a directory already // Verify that the destination does not exist as a directory already
final INode inode = iip.getLastINode(); final INode inode = iip.getLastINode();
@ -149,7 +148,8 @@ final class FSDirAppendOp {
fsd.writeUnlock(); fsd.writeUnlock();
} }
HdfsFileStatus stat = FSDirStatAndListingOp.getFileInfo(fsd, iip); HdfsFileStatus stat = FSDirStatAndListingOp.getFileInfo(fsd, src, false,
FSDirectory.isReservedRawName(srcArg));
if (lb != null) { if (lb != null) {
NameNode.stateChangeLog.debug( NameNode.stateChangeLog.debug(
"DIR* NameSystem.appendFile: file {} for {} at {} block {} block" "DIR* NameSystem.appendFile: file {} for {} at {} block {} block"

View File

@ -108,16 +108,16 @@ class FSDirStatAndListingOp {
if (!DFSUtil.isValidName(src)) { if (!DFSUtil.isValidName(src)) {
throw new InvalidPathException("Invalid file name: " + src); throw new InvalidPathException("Invalid file name: " + src);
} }
final INodesInPath iip;
if (fsd.isPermissionEnabled()) { if (fsd.isPermissionEnabled()) {
FSPermissionChecker pc = fsd.getPermissionChecker(); FSPermissionChecker pc = fsd.getPermissionChecker();
iip = fsd.resolvePath(pc, srcArg, resolveLink); final INodesInPath iip = fsd.resolvePath(pc, srcArg, resolveLink);
src = iip.getPath();
fsd.checkPermission(pc, iip, false, null, null, null, null, false); fsd.checkPermission(pc, iip, false, null, null, null, null, false);
} else { } else {
src = FSDirectory.resolvePath(srcArg, fsd); src = FSDirectory.resolvePath(srcArg, fsd);
iip = fsd.getINodesInPath(src, resolveLink);
} }
return getFileInfo(fsd, iip); return getFileInfo(fsd, src, FSDirectory.isReservedRawName(srcArg),
resolveLink);
} }
/** /**
@ -230,6 +230,7 @@ class FSDirStatAndListingOp {
String src, byte[] startAfter, boolean needLocation, boolean isSuperUser) String src, byte[] startAfter, boolean needLocation, boolean isSuperUser)
throws IOException { throws IOException {
String srcs = FSDirectory.normalizePath(src); String srcs = FSDirectory.normalizePath(src);
final boolean isRawPath = FSDirectory.isReservedRawName(src);
if (FSDirectory.isExactReservedName(srcs)) { if (FSDirectory.isExactReservedName(srcs)) {
return getReservedListing(fsd); return getReservedListing(fsd);
} }
@ -256,7 +257,7 @@ class FSDirStatAndListingOp {
return new DirectoryListing( return new DirectoryListing(
new HdfsFileStatus[]{ createFileStatus( new HdfsFileStatus[]{ createFileStatus(
fsd, HdfsFileStatus.EMPTY_NAME, nodeAttrs, fsd, HdfsFileStatus.EMPTY_NAME, nodeAttrs,
needLocation, parentStoragePolicy, iip) needLocation, parentStoragePolicy, snapshot, isRawPath, iip)
}, 0); }, 0);
} }
@ -281,7 +282,7 @@ class FSDirStatAndListingOp {
cur.getLocalNameBytes()); cur.getLocalNameBytes());
listing[i] = createFileStatus(fsd, cur.getLocalNameBytes(), nodeAttrs, listing[i] = createFileStatus(fsd, cur.getLocalNameBytes(), nodeAttrs,
needLocation, getStoragePolicyID(curPolicy, parentStoragePolicy), needLocation, getStoragePolicyID(curPolicy, parentStoragePolicy),
iipWithChild); snapshot, isRawPath, iipWithChild);
listingCnt++; listingCnt++;
if (needLocation) { if (needLocation) {
// Once we hit lsLimit locations, stop. // Once we hit lsLimit locations, stop.
@ -338,6 +339,7 @@ class FSDirStatAndListingOp {
listing[i] = createFileStatus( listing[i] = createFileStatus(
fsd, sRoot.getLocalNameBytes(), nodeAttrs, fsd, sRoot.getLocalNameBytes(), nodeAttrs,
HdfsConstants.BLOCK_STORAGE_POLICY_ID_UNSPECIFIED, HdfsConstants.BLOCK_STORAGE_POLICY_ID_UNSPECIFIED,
Snapshot.CURRENT_STATE_ID, false,
INodesInPath.fromINode(sRoot)); INodesInPath.fromINode(sRoot));
} }
return new DirectoryListing( return new DirectoryListing(
@ -361,8 +363,10 @@ class FSDirStatAndListingOp {
* @return object containing information regarding the file * @return object containing information regarding the file
* or null if file not found * or null if file not found
*/ */
static HdfsFileStatus getFileInfo(FSDirectory fsd, static HdfsFileStatus getFileInfo(
INodesInPath iip, boolean includeStoragePolicy) throws IOException { FSDirectory fsd, String path, INodesInPath iip, boolean isRawPath,
boolean includeStoragePolicy)
throws IOException {
fsd.readLock(); fsd.readLock();
try { try {
final INode node = iip.getLastINode(); final INode node = iip.getLastINode();
@ -373,32 +377,36 @@ class FSDirStatAndListingOp {
byte policyId = includeStoragePolicy && !node.isSymlink() ? byte policyId = includeStoragePolicy && !node.isSymlink() ?
node.getStoragePolicyID() : node.getStoragePolicyID() :
HdfsConstants.BLOCK_STORAGE_POLICY_ID_UNSPECIFIED; HdfsConstants.BLOCK_STORAGE_POLICY_ID_UNSPECIFIED;
INodeAttributes nodeAttrs = getINodeAttributes(fsd, iip.getPath(), INodeAttributes nodeAttrs = getINodeAttributes(fsd, path,
HdfsFileStatus.EMPTY_NAME, HdfsFileStatus.EMPTY_NAME,
node, iip.getPathSnapshotId()); node, iip.getPathSnapshotId());
return createFileStatus(fsd, HdfsFileStatus.EMPTY_NAME, nodeAttrs, return createFileStatus(fsd, HdfsFileStatus.EMPTY_NAME, nodeAttrs,
policyId, iip); policyId, iip.getPathSnapshotId(), isRawPath, iip);
} finally { } finally {
fsd.readUnlock(); fsd.readUnlock();
} }
} }
static HdfsFileStatus getFileInfo(FSDirectory fsd, INodesInPath iip) static HdfsFileStatus getFileInfo(
FSDirectory fsd, String src, boolean resolveLink, boolean isRawPath)
throws IOException { throws IOException {
fsd.readLock();
try {
HdfsFileStatus status = null;
final INodesInPath iip = fsd.getINodesInPath(src, resolveLink);
if (FSDirectory.isExactReservedName(iip.getPathComponents())) { if (FSDirectory.isExactReservedName(iip.getPathComponents())) {
return FSDirectory.DOT_RESERVED_STATUS; status = FSDirectory.DOT_RESERVED_STATUS;
} } else if (iip.isDotSnapshotDir()) {
if (iip.isDotSnapshotDir()) {
if (fsd.getINode4DotSnapshot(iip) != null) { if (fsd.getINode4DotSnapshot(iip) != null) {
return new HdfsFileStatus(0, true, 0, 0, 0, 0, null, null, null, null, status = FSDirectory.DOT_SNAPSHOT_DIR_STATUS;
HdfsFileStatus.EMPTY_NAME, -1L, 0, null,
HdfsConstants.BLOCK_STORAGE_POLICY_ID_UNSPECIFIED, null);
} }
return null; } else {
status = getFileInfo(fsd, src, iip, isRawPath, true);
}
return status;
} finally {
fsd.readUnlock();
} }
return getFileInfo(fsd, iip, true);
} }
/** /**
@ -415,12 +423,15 @@ class FSDirStatAndListingOp {
*/ */
private static HdfsFileStatus createFileStatus( private static HdfsFileStatus createFileStatus(
FSDirectory fsd, byte[] path, INodeAttributes nodeAttrs, FSDirectory fsd, byte[] path, INodeAttributes nodeAttrs,
boolean needLocation, byte storagePolicy, INodesInPath iip) boolean needLocation, byte storagePolicy, int snapshot, boolean isRawPath,
INodesInPath iip)
throws IOException { throws IOException {
if (needLocation) { if (needLocation) {
return createLocatedFileStatus(fsd, path, nodeAttrs, storagePolicy, iip); return createLocatedFileStatus(fsd, path, nodeAttrs, storagePolicy,
snapshot, isRawPath, iip);
} else { } else {
return createFileStatus(fsd, path, nodeAttrs, storagePolicy, iip); return createFileStatus(fsd, path, nodeAttrs, storagePolicy,
snapshot, isRawPath, iip);
} }
} }
@ -434,7 +445,8 @@ class FSDirStatAndListingOp {
INodesInPath iip) throws IOException { INodesInPath iip) throws IOException {
INodeAttributes nodeAttrs = getINodeAttributes( INodeAttributes nodeAttrs = getINodeAttributes(
fsd, fullPath, path, iip.getLastINode(), snapshot); fsd, fullPath, path, iip.getLastINode(), snapshot);
return createFileStatus(fsd, path, nodeAttrs, storagePolicy, iip); return createFileStatus(fsd, path, nodeAttrs, storagePolicy,
snapshot, isRawPath, iip);
} }
/** /**
@ -442,15 +454,14 @@ class FSDirStatAndListingOp {
* @param iip the INodesInPath containing the target INode and its ancestors * @param iip the INodesInPath containing the target INode and its ancestors
*/ */
static HdfsFileStatus createFileStatus( static HdfsFileStatus createFileStatus(
FSDirectory fsd, byte[] path, INodeAttributes nodeAttrs, FSDirectory fsd, byte[] path,
byte storagePolicy, INodesInPath iip) throws IOException { INodeAttributes nodeAttrs, byte storagePolicy, int snapshot,
boolean isRawPath, INodesInPath iip) throws IOException {
long size = 0; // length is zero for directories long size = 0; // length is zero for directories
short replication = 0; short replication = 0;
long blocksize = 0; long blocksize = 0;
final boolean isEncrypted; final boolean isEncrypted;
final INode node = iip.getLastINode(); final INode node = iip.getLastINode();
final int snapshot = iip.getPathSnapshotId();
final boolean isRawPath = iip.isRaw();
final FileEncryptionInfo feInfo = isRawPath ? null : FSDirEncryptionZoneOp final FileEncryptionInfo feInfo = isRawPath ? null : FSDirEncryptionZoneOp
.getFileEncryptionInfo(fsd, node, snapshot, iip); .getFileEncryptionInfo(fsd, node, snapshot, iip);
@ -500,9 +511,10 @@ class FSDirStatAndListingOp {
* Create FileStatus with location info by file INode * Create FileStatus with location info by file INode
* @param iip the INodesInPath containing the target INode and its ancestors * @param iip the INodesInPath containing the target INode and its ancestors
*/ */
private static HdfsFileStatus createLocatedFileStatus( private static HdfsLocatedFileStatus createLocatedFileStatus(
FSDirectory fsd, byte[] path, INodeAttributes nodeAttrs, FSDirectory fsd, byte[] path, INodeAttributes nodeAttrs,
byte storagePolicy, INodesInPath iip) throws IOException { byte storagePolicy, int snapshot,
boolean isRawPath, INodesInPath iip) throws IOException {
assert fsd.hasReadLock(); assert fsd.hasReadLock();
long size = 0; // length is zero for directories long size = 0; // length is zero for directories
short replication = 0; short replication = 0;
@ -510,8 +522,6 @@ class FSDirStatAndListingOp {
LocatedBlocks loc = null; LocatedBlocks loc = null;
final boolean isEncrypted; final boolean isEncrypted;
final INode node = iip.getLastINode(); final INode node = iip.getLastINode();
final int snapshot = iip.getPathSnapshotId();
final boolean isRawPath = iip.isRaw();
final FileEncryptionInfo feInfo = isRawPath ? null : FSDirEncryptionZoneOp final FileEncryptionInfo feInfo = isRawPath ? null : FSDirEncryptionZoneOp
.getFileEncryptionInfo(fsd, node, snapshot, iip); .getFileEncryptionInfo(fsd, node, snapshot, iip);

View File

@ -340,6 +340,7 @@ class FSDirWriteFileOp {
version = ezInfo.protocolVersion; version = ezInfo.protocolVersion;
} }
boolean isRawPath = FSDirectory.isReservedRawName(src);
FSDirectory fsd = fsn.getFSDirectory(); FSDirectory fsd = fsn.getFSDirectory();
INodesInPath iip = fsd.resolvePathForWrite(pc, src); INodesInPath iip = fsd.resolvePathForWrite(pc, src);
src = iip.getPath(); src = iip.getPath();
@ -443,7 +444,7 @@ class FSDirWriteFileOp {
NameNode.stateChangeLog.debug("DIR* NameSystem.startFile: added " + NameNode.stateChangeLog.debug("DIR* NameSystem.startFile: added " +
src + " inode " + newNode.getId() + " " + holder); src + " inode " + newNode.getId() + " " + holder);
} }
return FSDirStatAndListingOp.getFileInfo(fsd, iip); return FSDirStatAndListingOp.getFileInfo(fsd, src, false, isRawPath);
} }
static EncryptionKeyInfo getEncryptionKeyInfo(FSNamesystem fsn, static EncryptionKeyInfo getEncryptionKeyInfo(FSNamesystem fsn,

View File

@ -531,24 +531,21 @@ public class FSDirectory implements Closeable {
* @throws FileNotFoundException * @throws FileNotFoundException
* @throws AccessControlException * @throws AccessControlException
*/ */
@VisibleForTesting INodesInPath resolvePath(FSPermissionChecker pc, String src)
public INodesInPath resolvePath(FSPermissionChecker pc, String src)
throws UnresolvedLinkException, FileNotFoundException, throws UnresolvedLinkException, FileNotFoundException,
AccessControlException { AccessControlException {
return resolvePath(pc, src, true); return resolvePath(pc, src, true);
} }
@VisibleForTesting INodesInPath resolvePath(FSPermissionChecker pc, String src,
public INodesInPath resolvePath(FSPermissionChecker pc, String src,
boolean resolveLink) throws UnresolvedLinkException, boolean resolveLink) throws UnresolvedLinkException,
FileNotFoundException, AccessControlException { FileNotFoundException, AccessControlException {
byte[][] components = INode.getPathComponents(src); byte[][] components = INode.getPathComponents(src);
boolean isRaw = isReservedRawName(components); if (isPermissionEnabled && pc != null && isReservedRawName(components)) {
if (isPermissionEnabled && pc != null && isRaw) {
pc.checkSuperuserPrivilege(); pc.checkSuperuserPrivilege();
} }
components = resolveComponents(components, this); components = resolveComponents(components, this);
return INodesInPath.resolve(rootDir, components, isRaw, resolveLink); return INodesInPath.resolve(rootDir, components, resolveLink);
} }
INodesInPath resolvePathForWrite(FSPermissionChecker pc, String src) INodesInPath resolvePathForWrite(FSPermissionChecker pc, String src)
@ -1665,7 +1662,8 @@ public class FSDirectory implements Closeable {
HdfsFileStatus getAuditFileInfo(INodesInPath iip) HdfsFileStatus getAuditFileInfo(INodesInPath iip)
throws IOException { throws IOException {
return (namesystem.isAuditEnabled() && namesystem.isExternalInvocation()) return (namesystem.isAuditEnabled() && namesystem.isExternalInvocation())
? FSDirStatAndListingOp.getFileInfo(this, iip, false) : null; ? FSDirStatAndListingOp.getFileInfo(this, iip.getPath(), iip, false,
false) : null;
} }
/** /**

View File

@ -126,12 +126,6 @@ public class INodesInPath {
static INodesInPath resolve(final INodeDirectory startingDir, static INodesInPath resolve(final INodeDirectory startingDir,
final byte[][] components, final boolean resolveLink) final byte[][] components, final boolean resolveLink)
throws UnresolvedLinkException { throws UnresolvedLinkException {
return resolve(startingDir, components, false, resolveLink);
}
static INodesInPath resolve(final INodeDirectory startingDir,
final byte[][] components, final boolean isRaw,
final boolean resolveLink) throws UnresolvedLinkException {
Preconditions.checkArgument(startingDir.compareTo(components[0]) == 0); Preconditions.checkArgument(startingDir.compareTo(components[0]) == 0);
INode curNode = startingDir; INode curNode = startingDir;
@ -231,7 +225,7 @@ public class INodesInPath {
System.arraycopy(inodes, 0, newNodes, 0, newNodes.length); System.arraycopy(inodes, 0, newNodes, 0, newNodes.length);
inodes = newNodes; inodes = newNodes;
} }
return new INodesInPath(inodes, components, isRaw, isSnapshot, snapshotId); return new INodesInPath(inodes, components, isSnapshot, snapshotId);
} }
private static boolean shouldUpdateLatestId(int sid, int snapshotId) { private static boolean shouldUpdateLatestId(int sid, int snapshotId) {
@ -255,8 +249,7 @@ public class INodesInPath {
INode[] inodes = new INode[iip.inodes.length]; INode[] inodes = new INode[iip.inodes.length];
System.arraycopy(iip.inodes, 0, inodes, 0, inodes.length); System.arraycopy(iip.inodes, 0, inodes, 0, inodes.length);
inodes[pos] = inode; inodes[pos] = inode;
return new INodesInPath(inodes, iip.path, iip.isRaw, return new INodesInPath(inodes, iip.path, iip.isSnapshot, iip.snapshotId);
iip.isSnapshot, iip.snapshotId);
} }
/** /**
@ -274,8 +267,7 @@ public class INodesInPath {
byte[][] path = new byte[iip.path.length + 1][]; byte[][] path = new byte[iip.path.length + 1][];
System.arraycopy(iip.path, 0, path, 0, path.length - 1); System.arraycopy(iip.path, 0, path, 0, path.length - 1);
path[path.length - 1] = childName; path[path.length - 1] = childName;
return new INodesInPath(inodes, path, iip.isRaw, return new INodesInPath(inodes, path, iip.isSnapshot, iip.snapshotId);
iip.isSnapshot, iip.snapshotId);
} }
private final byte[][] path; private final byte[][] path;
@ -287,13 +279,6 @@ public class INodesInPath {
* true if this path corresponds to a snapshot * true if this path corresponds to a snapshot
*/ */
private final boolean isSnapshot; private final boolean isSnapshot;
/**
* true if this is a /.reserved/raw path. path component resolution strips
* it from the path so need to track it separately.
*/
private final boolean isRaw;
/** /**
* For snapshot paths, it is the id of the snapshot; or * For snapshot paths, it is the id of the snapshot; or
* {@link Snapshot#CURRENT_STATE_ID} if the snapshot does not exist. For * {@link Snapshot#CURRENT_STATE_ID} if the snapshot does not exist. For
@ -302,18 +287,17 @@ public class INodesInPath {
*/ */
private final int snapshotId; private final int snapshotId;
private INodesInPath(INode[] inodes, byte[][] path, boolean isRaw, private INodesInPath(INode[] inodes, byte[][] path, boolean isSnapshot,
boolean isSnapshot,int snapshotId) { int snapshotId) {
Preconditions.checkArgument(inodes != null && path != null); Preconditions.checkArgument(inodes != null && path != null);
this.inodes = inodes; this.inodes = inodes;
this.path = path; this.path = path;
this.isRaw = isRaw;
this.isSnapshot = isSnapshot; this.isSnapshot = isSnapshot;
this.snapshotId = snapshotId; this.snapshotId = snapshotId;
} }
private INodesInPath(INode[] inodes, byte[][] path) { private INodesInPath(INode[] inodes, byte[][] path) {
this(inodes, path, false, false, CURRENT_STATE_ID); this(inodes, path, false, CURRENT_STATE_ID);
} }
/** /**
@ -416,7 +400,7 @@ public class INodesInPath {
final byte[][] apath = new byte[length][]; final byte[][] apath = new byte[length][];
System.arraycopy(this.inodes, 0, anodes, 0, length); System.arraycopy(this.inodes, 0, anodes, 0, length);
System.arraycopy(this.path, 0, apath, 0, length); System.arraycopy(this.path, 0, apath, 0, length);
return new INodesInPath(anodes, apath, isRaw, false, snapshotId); return new INodesInPath(anodes, apath, false, snapshotId);
} }
/** /**
@ -444,7 +428,7 @@ public class INodesInPath {
byte[][] existingPath = new byte[i][]; byte[][] existingPath = new byte[i][];
System.arraycopy(inodes, 0, existing, 0, i); System.arraycopy(inodes, 0, existing, 0, i);
System.arraycopy(path, 0, existingPath, 0, i); System.arraycopy(path, 0, existingPath, 0, i);
return new INodesInPath(existing, existingPath, isRaw, false, snapshotId); return new INodesInPath(existing, existingPath, false, snapshotId);
} }
/** /**
@ -454,20 +438,10 @@ public class INodesInPath {
return this.isSnapshot; return this.isSnapshot;
} }
/**
* @return if .snapshot is the last path component.
*/
boolean isDotSnapshotDir() { boolean isDotSnapshotDir() {
return isDotSnapshotDir(getLastLocalName()); return isDotSnapshotDir(getLastLocalName());
} }
/**
* @return if this is a /.reserved/raw path.
*/
public boolean isRaw() {
return isRaw;
}
private static String toString(INode inode) { private static String toString(INode inode) {
return inode == null? null: inode.getLocalName(); return inode == null? null: inode.getLocalName();
} }

View File

@ -36,7 +36,6 @@ import org.apache.hadoop.hdfs.client.CreateEncryptionZoneFlag;
import org.apache.hadoop.hdfs.client.HdfsAdmin; import org.apache.hadoop.hdfs.client.HdfsAdmin;
import org.apache.hadoop.hdfs.server.namenode.EncryptionZoneManager; import org.apache.hadoop.hdfs.server.namenode.EncryptionZoneManager;
import org.apache.hadoop.hdfs.server.namenode.FSDirectory; import org.apache.hadoop.hdfs.server.namenode.FSDirectory;
import org.apache.hadoop.hdfs.server.namenode.INodesInPath;
import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.AccessControlException;
import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.UserGroupInformation;
import org.apache.log4j.Level; import org.apache.log4j.Level;
@ -50,8 +49,6 @@ import static org.apache.hadoop.hdfs.DFSTestUtil.verifyFilesNotEqual;
import static org.apache.hadoop.test.GenericTestUtils.assertExceptionContains; import static org.apache.hadoop.test.GenericTestUtils.assertExceptionContains;
import static org.apache.hadoop.test.GenericTestUtils.assertMatches; import static org.apache.hadoop.test.GenericTestUtils.assertMatches;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
public class TestReservedRawPaths { public class TestReservedRawPaths {
@ -102,24 +99,6 @@ public class TestReservedRawPaths {
} }
} }
/**
* Verify resolving path will return an iip that tracks if the original
* path was a raw path.
*/
@Test(timeout = 120000)
public void testINodesInPath() throws IOException {
FSDirectory fsd = cluster.getNamesystem().getFSDirectory();
final String path = "/path";
INodesInPath iip = fsd.resolvePath(null, path);
assertFalse(iip.isRaw());
assertEquals(path, iip.getPath());
iip = fsd.resolvePath(null, "/.reserved/raw" + path);
assertTrue(iip.isRaw());
assertEquals(path, iip.getPath());
}
/** /**
* Basic read/write tests of raw files. * Basic read/write tests of raw files.
* Create a non-encrypted file * Create a non-encrypted file