HDFS-10956. Remove rename/delete performance penalty when not using snapshots. Contributed by Daryn Sharp.
This commit is contained in:
parent
88b9444a81
commit
44f48ee96e
|
@ -57,7 +57,7 @@ class FSDirDeleteOp {
|
|||
try {
|
||||
if (deleteAllowed(iip, iip.getPath()) ) {
|
||||
List<INodeDirectory> snapshottableDirs = new ArrayList<>();
|
||||
FSDirSnapshotOp.checkSnapshot(iip.getLastINode(), snapshottableDirs);
|
||||
FSDirSnapshotOp.checkSnapshot(fsd, iip, snapshottableDirs);
|
||||
ReclaimContext context = new ReclaimContext(
|
||||
fsd.getBlockStoragePolicySuite(), collectedBlocks, removedINodes,
|
||||
removedUCFiles);
|
||||
|
@ -140,7 +140,7 @@ class FSDirDeleteOp {
|
|||
return;
|
||||
}
|
||||
List<INodeDirectory> snapshottableDirs = new ArrayList<>();
|
||||
FSDirSnapshotOp.checkSnapshot(iip.getLastINode(), snapshottableDirs);
|
||||
FSDirSnapshotOp.checkSnapshot(fsd, iip, snapshottableDirs);
|
||||
boolean filesRemoved = unprotectedDelete(fsd, iip,
|
||||
new ReclaimContext(fsd.getBlockStoragePolicySuite(),
|
||||
collectedBlocks, removedINodes, removedUCFiles),
|
||||
|
|
|
@ -156,7 +156,7 @@ class FSDirRenameOp {
|
|||
assert fsd.hasWriteLock();
|
||||
final INode srcInode = srcIIP.getLastINode();
|
||||
try {
|
||||
validateRenameSource(srcIIP);
|
||||
validateRenameSource(fsd, srcIIP);
|
||||
} catch (SnapshotException e) {
|
||||
throw e;
|
||||
} catch (IOException ignored) {
|
||||
|
@ -365,7 +365,7 @@ class FSDirRenameOp {
|
|||
final String dst = dstIIP.getPath();
|
||||
final String error;
|
||||
final INode srcInode = srcIIP.getLastINode();
|
||||
validateRenameSource(srcIIP);
|
||||
validateRenameSource(fsd, srcIIP);
|
||||
|
||||
// validate the destination
|
||||
if (dst.equals(src)) {
|
||||
|
@ -387,7 +387,7 @@ class FSDirRenameOp {
|
|||
List<INodeDirectory> snapshottableDirs = new ArrayList<>();
|
||||
if (dstInode != null) { // Destination exists
|
||||
validateOverwrite(src, dst, overwrite, srcInode, dstInode);
|
||||
FSDirSnapshotOp.checkSnapshot(dstInode, snapshottableDirs);
|
||||
FSDirSnapshotOp.checkSnapshot(fsd, dstIIP, snapshottableDirs);
|
||||
}
|
||||
|
||||
INode dstParent = dstIIP.getINode(-2);
|
||||
|
@ -559,8 +559,8 @@ class FSDirRenameOp {
|
|||
}
|
||||
}
|
||||
|
||||
private static void validateRenameSource(INodesInPath srcIIP)
|
||||
throws IOException {
|
||||
private static void validateRenameSource(FSDirectory fsd,
|
||||
INodesInPath srcIIP) throws IOException {
|
||||
String error;
|
||||
final INode srcInode = srcIIP.getLastINode();
|
||||
// validate source
|
||||
|
@ -578,7 +578,7 @@ class FSDirRenameOp {
|
|||
}
|
||||
// srcInode and its subtree cannot contain snapshottable directories with
|
||||
// snapshots
|
||||
FSDirSnapshotOp.checkSnapshot(srcInode, null);
|
||||
FSDirSnapshotOp.checkSnapshot(fsd, srcIIP, null);
|
||||
}
|
||||
|
||||
private static class RenameOperation {
|
||||
|
|
|
@ -35,7 +35,6 @@ import org.apache.hadoop.util.ChunkedArrayList;
|
|||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.ListIterator;
|
||||
import java.util.List;
|
||||
|
||||
class FSDirSnapshotOp {
|
||||
|
@ -252,7 +251,7 @@ class FSDirSnapshotOp {
|
|||
* @param snapshottableDirs The list of directories that are snapshottable
|
||||
* but do not have snapshots yet
|
||||
*/
|
||||
static void checkSnapshot(
|
||||
private static void checkSnapshot(
|
||||
INode target, List<INodeDirectory> snapshottableDirs)
|
||||
throws SnapshotException {
|
||||
if (target.isDirectory()) {
|
||||
|
@ -276,4 +275,23 @@ class FSDirSnapshotOp {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the given path (or one of its descendants) is snapshottable and
|
||||
* already has snapshots.
|
||||
*
|
||||
* @param fsd the FSDirectory
|
||||
* @param iip inodes of the path
|
||||
* @param snapshottableDirs The list of directories that are snapshottable
|
||||
* but do not have snapshots yet
|
||||
*/
|
||||
static void checkSnapshot(FSDirectory fsd, INodesInPath iip,
|
||||
List<INodeDirectory> snapshottableDirs) throws SnapshotException {
|
||||
// avoid the performance penalty of recursing the tree if snapshots
|
||||
// are not in use
|
||||
SnapshotManager sm = fsd.getFSNamesystem().getSnapshotManager();
|
||||
if (sm.getNumSnapshottableDirs() > 0) {
|
||||
checkSnapshot(iip.getLastINode(), snapshottableDirs);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -22,6 +22,7 @@ import static org.junit.Assert.assertNull;
|
|||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import java.io.FileNotFoundException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
import org.apache.hadoop.conf.Configuration;
|
||||
|
@ -30,12 +31,15 @@ import org.apache.hadoop.hdfs.DFSTestUtil;
|
|||
import org.apache.hadoop.hdfs.DFSUtil;
|
||||
import org.apache.hadoop.hdfs.DistributedFileSystem;
|
||||
import org.apache.hadoop.hdfs.MiniDFSCluster;
|
||||
import org.apache.hadoop.hdfs.protocol.SnapshotException;
|
||||
import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot;
|
||||
import org.apache.hadoop.hdfs.server.namenode.snapshot.SnapshotManager;
|
||||
import org.junit.AfterClass;
|
||||
import org.junit.Assert;
|
||||
import org.junit.Before;
|
||||
import org.junit.BeforeClass;
|
||||
import org.junit.Test;
|
||||
import org.mockito.Mockito;
|
||||
|
||||
/** Test snapshot related operations. */
|
||||
public class TestSnapshotPathINodes {
|
||||
|
@ -426,4 +430,16 @@ public class TestSnapshotPathINodes {
|
|||
hdfs.deleteSnapshot(sub1, "s3");
|
||||
hdfs.disallowSnapshot(sub1);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testShortCircuitSnapshotSearch() throws SnapshotException {
|
||||
FSNamesystem fsn = cluster.getNamesystem();
|
||||
SnapshotManager sm = fsn.getSnapshotManager();
|
||||
assertEquals(0, sm.getNumSnapshottableDirs());
|
||||
|
||||
INodesInPath iip = Mockito.mock(INodesInPath.class);
|
||||
List<INodeDirectory> snapDirs = new ArrayList<>();
|
||||
FSDirSnapshotOp.checkSnapshot(fsn.getFSDirectory(), iip, snapDirs);
|
||||
Mockito.verifyZeroInteractions(iip);
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue