From 0f78c50ea7f25515f43a7570fe67a6604e8772ad Mon Sep 17 00:00:00 2001 From: Tsz-wo Sze Date: Sat, 13 Apr 2013 21:41:33 +0000 Subject: [PATCH] HDFS-4692. Use timestamp as default snapshot names. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/HDFS-2802@1467706 13f79535-47bb-0310-9956-ffa450edef68 --- .../java/org/apache/hadoop/fs/FileSystem.java | 12 ++++++- .../hadoop/fs/shell/SnapshotCommands.java | 21 ++++++----- .../hadoop-hdfs/CHANGES.HDFS-2802.txt | 2 ++ .../org/apache/hadoop/hdfs/DFSClient.java | 5 +-- .../hadoop/hdfs/DistributedFileSystem.java | 4 +-- .../hadoop/hdfs/protocol/ClientProtocol.java | 3 +- ...amenodeProtocolServerSideTranslatorPB.java | 20 ++++++----- .../ClientNamenodeProtocolTranslatorPB.java | 17 +++++---- .../hdfs/server/namenode/FSNamesystem.java | 31 ++++++++-------- .../server/namenode/NameNodeRpcServer.java | 7 ++-- .../snapshot/INodeDirectorySnapshottable.java | 2 +- .../server/namenode/snapshot/Snapshot.java | 35 ++++++++++++++----- .../namenode/snapshot/SnapshotManager.java | 4 +-- .../main/proto/ClientNamenodeProtocol.proto | 5 +-- .../snapshot/TestNestedSnapshots.java | 17 +++++++-- 15 files changed, 119 insertions(+), 66 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java index f4ba24bacab..19dd348d595 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java @@ -2265,12 +2265,22 @@ public void setTimes(Path p, long mtime, long atime ) throws IOException { } + /** + * Create a snapshot with a default name. + * @param path The directory where snapshots will be taken. + * @return the snapshot path. + */ + public final Path createSnapshot(Path path) throws IOException { + return createSnapshot(path, null); + } + /** * Create a snapshot * @param path The directory where snapshots will be taken. * @param snapshotName The name of the snapshot + * @return the snapshot path. */ - public void createSnapshot(Path path, String snapshotName) + public Path createSnapshot(Path path, String snapshotName) throws IOException { throw new UnsupportedOperationException(getClass().getSimpleName() + " doesn't support createSnapshot"); diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/SnapshotCommands.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/SnapshotCommands.java index e0621a739a6..79da1078ef2 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/SnapshotCommands.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/SnapshotCommands.java @@ -49,10 +49,10 @@ public static void registerCommands(CommandFactory factory) { */ public static class CreateSnapshot extends FsCommand { public static final String NAME = CREATE_SNAPSHOT; - public static final String USAGE = " "; + public static final String USAGE = " []"; public static final String DESCRIPTION = "Create a snapshot on a directory"; - private String snapshotName; + private String snapshotName = null; @Override protected void processPath(PathData item) throws IOException { @@ -63,12 +63,15 @@ protected void processPath(PathData item) throws IOException { @Override protected void processOptions(LinkedList args) throws IOException { - if (args.size() != 2) { - throw new IOException("args number not 2:" + args.size()); + if (args.size() == 0) { + throw new IllegalArgumentException(" is missing."); + } + if (args.size() > 2) { + throw new IllegalArgumentException("Too many arguements."); + } + if (args.size() == 2) { + snapshotName = args.removeLast(); } - snapshotName = args.removeLast(); - // TODO: name length check - } @Override @@ -108,8 +111,6 @@ protected void processOptions(LinkedList args) throws IOException { throw new IOException("args number not 2: " + args.size()); } snapshotName = args.removeLast(); - // TODO: name length check - } @Override @@ -151,8 +152,6 @@ protected void processOptions(LinkedList args) throws IOException { } newName = args.removeLast(); oldName = args.removeLast(); - - // TODO: new name length check } @Override diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt index f3a02d155a4..efcfd9f6875 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-2802.txt @@ -234,3 +234,5 @@ Branch-2802 Snapshot (Unreleased) HDFS-4675. Fix rename across snapshottable directories. (Jing Zhao via szetszwo) + + HDFS-4692. Use timestamp as default snapshot names. (szetszwo) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java index 62d466fc7d5..b747567b3dd 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java @@ -2021,12 +2021,13 @@ public boolean setSafeMode(SafeModeAction action, boolean isChecked) throws IOEx * * @param snapshotRoot The directory where the snapshot is to be taken * @param snapshotName Name of the snapshot + * @return the snapshot path. * @see ClientProtocol#createSnapshot(String, String) */ - public void createSnapshot(String snapshotRoot, String snapshotName) + public String createSnapshot(String snapshotRoot, String snapshotName) throws IOException { checkOpen(); - namenode.createSnapshot(snapshotRoot, snapshotName); + return namenode.createSnapshot(snapshotRoot, snapshotName); } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java index 1c320926cfc..eedbd0548f8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java @@ -932,9 +932,9 @@ public void disallowSnapshot(String path) throws IOException { } @Override - public void createSnapshot(Path path, String snapshotName) + public Path createSnapshot(Path path, String snapshotName) throws IOException { - dfs.createSnapshot(getPathName(path), snapshotName); + return new Path(dfs.createSnapshot(getPathName(path), snapshotName)); } @Override diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java index 5ed8426949e..7fbb79090a5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java @@ -1000,9 +1000,10 @@ public void cancelDelegationToken(Token token) * Create a snapshot * @param snapshotRoot the path that is being snapshotted * @param snapshotName name of the snapshot created + * @return the snapshot path. * @throws IOException */ - public void createSnapshot(String snapshotRoot, String snapshotName) + public String createSnapshot(String snapshotRoot, String snapshotName) throws IOException; /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolServerSideTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolServerSideTranslatorPB.java index 9f5af213e68..46f14605717 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolServerSideTranslatorPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolServerSideTranslatorPB.java @@ -165,8 +165,6 @@ public class ClientNamenodeProtocolServerSideTranslatorPB implements ClientNamenodeProtocolPB { final private ClientProtocol server; - static final CreateSnapshotResponseProto VOID_CREATE_SNAPSHOT_RESPONSE = - CreateSnapshotResponseProto.newBuilder().build(); static final DeleteSnapshotResponseProto VOID_DELETE_SNAPSHOT_RESPONSE = DeleteSnapshotResponseProto.newBuilder().build(); static final RenameSnapshotResponseProto VOID_RENAME_SNAPSHOT_RESPONSE = @@ -898,22 +896,26 @@ public GetDataEncryptionKeyResponseProto getDataEncryptionKey( @Override public CreateSnapshotResponseProto createSnapshot(RpcController controller, - CreateSnapshotRequestProto request) throws ServiceException { + CreateSnapshotRequestProto req) throws ServiceException { try { - server.createSnapshot(request.getSnapshotRoot(), - request.getSnapshotName()); + final CreateSnapshotResponseProto.Builder builder + = CreateSnapshotResponseProto.newBuilder(); + final String snapshotPath = server.createSnapshot(req.getSnapshotRoot(), + req.hasSnapshotName()? req.getSnapshotName(): null); + if (snapshotPath != null) { + builder.setSnapshotPath(snapshotPath); + } + return builder.build(); } catch (IOException e) { throw new ServiceException(e); } - return VOID_CREATE_SNAPSHOT_RESPONSE; } @Override public DeleteSnapshotResponseProto deleteSnapshot(RpcController controller, - DeleteSnapshotRequestProto request) throws ServiceException { + DeleteSnapshotRequestProto req) throws ServiceException { try { - server - .deleteSnapshot(request.getSnapshotRoot(), request.getSnapshotName()); + server.deleteSnapshot(req.getSnapshotRoot(), req.getSnapshotName()); return VOID_DELETE_SNAPSHOT_RESPONSE; } catch (IOException e) { throw new ServiceException(e); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolTranslatorPB.java index 92349bed172..ce609a51d37 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolTranslatorPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/ClientNamenodeProtocolTranslatorPB.java @@ -40,13 +40,13 @@ import org.apache.hadoop.hdfs.protocol.DatanodeInfo; import org.apache.hadoop.hdfs.protocol.DirectoryListing; import org.apache.hadoop.hdfs.protocol.ExtendedBlock; -import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport; import org.apache.hadoop.hdfs.protocol.HdfsConstants.DatanodeReportType; import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction; import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; import org.apache.hadoop.hdfs.protocol.LocatedBlock; import org.apache.hadoop.hdfs.protocol.LocatedBlocks; import org.apache.hadoop.hdfs.protocol.NSQuotaExceededException; +import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport; import org.apache.hadoop.hdfs.protocol.SnapshottableDirectoryStatus; import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.AbandonBlockRequestProto; import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.AddBlockRequestProto; @@ -56,8 +56,8 @@ import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.CompleteRequestProto; import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.ConcatRequestProto; import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.CreateRequestProto; -import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.CreateSnapshotRequestProto; import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.CreateResponseProto; +import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.CreateSnapshotRequestProto; import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.CreateSymlinkRequestProto; import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.DeleteRequestProto; import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.DeleteSnapshotRequestProto; @@ -112,7 +112,6 @@ import org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos.UpdatePipelineRequestProto; import org.apache.hadoop.hdfs.security.token.block.DataEncryptionKey; import org.apache.hadoop.hdfs.security.token.delegation.DelegationTokenIdentifier; -import org.apache.hadoop.hdfs.server.namenode.INodeId; import org.apache.hadoop.hdfs.server.namenode.NotReplicatedYetException; import org.apache.hadoop.hdfs.server.namenode.SafeModeException; import org.apache.hadoop.io.EnumSetWritable; @@ -882,12 +881,16 @@ public Object getUnderlyingProxyObject() { } @Override - public void createSnapshot(String snapshotRoot, String snapshotName) + public String createSnapshot(String snapshotRoot, String snapshotName) throws IOException { - CreateSnapshotRequestProto req = CreateSnapshotRequestProto.newBuilder() - .setSnapshotRoot(snapshotRoot).setSnapshotName(snapshotName).build(); + final CreateSnapshotRequestProto.Builder builder + = CreateSnapshotRequestProto.newBuilder().setSnapshotRoot(snapshotRoot); + if (snapshotName != null) { + builder.setSnapshotName(snapshotName); + } + final CreateSnapshotRequestProto req = builder.build(); try { - rpcProxy.createSnapshot(null, req); + return rpcProxy.createSnapshot(null, req).getSnapshotPath(); } catch (ServiceException e) { throw ProtobufHelper.getRemoteException(e); } 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 25928607d68..ee9a05776e0 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 @@ -5731,12 +5731,12 @@ public boolean isAvoidingStaleDataNodesForWrite() { .shouldAvoidStaleDataNodesForWrite(); } - public SnapshotManager getSnapshotManager() { + SnapshotManager getSnapshotManager() { return snapshotManager; } /** Allow snapshot on a directroy. */ - public void allowSnapshot(String path) throws SafeModeException, IOException { + void allowSnapshot(String path) throws SafeModeException, IOException { final FSPermissionChecker pc = getPermissionChecker(); writeLock(); try { @@ -5762,8 +5762,7 @@ public void allowSnapshot(String path) throws SafeModeException, IOException { } /** Disallow snapshot on a directory. */ - public void disallowSnapshot(String path) - throws SafeModeException, IOException { + void disallowSnapshot(String path) throws SafeModeException, IOException { final FSPermissionChecker pc = getPermissionChecker(); writeLock(); try { @@ -5793,10 +5792,11 @@ public void disallowSnapshot(String path) * @param snapshotRoot The directory path where the snapshot is taken * @param snapshotName The name of the snapshot */ - public void createSnapshot(String snapshotRoot, String snapshotName) + String createSnapshot(String snapshotRoot, String snapshotName) throws SafeModeException, IOException { final FSPermissionChecker pc = getPermissionChecker(); writeLock(); + final String snapshotPath; try { checkOperation(OperationCategory.WRITE); if (isInSafeMode()) { @@ -5805,9 +5805,13 @@ public void createSnapshot(String snapshotRoot, String snapshotName) } checkOwner(pc, snapshotRoot); + if (snapshotName == null || snapshotName.isEmpty()) { + snapshotName = Snapshot.generateDefaultSnapshotName(); + } + dir.verifyMaxComponentLength(snapshotName, snapshotRoot, 0); dir.writeLock(); try { - snapshotManager.createSnapshot(snapshotRoot, snapshotName); + snapshotPath = snapshotManager.createSnapshot(snapshotRoot, snapshotName); } finally { dir.writeUnlock(); } @@ -5818,11 +5822,9 @@ public void createSnapshot(String snapshotRoot, String snapshotName) getEditLog().logSync(); if (auditLog.isInfoEnabled() && isExternalInvocation()) { - Path rootPath = new Path(snapshotRoot, HdfsConstants.DOT_SNAPSHOT_DIR - + Path.SEPARATOR + snapshotName); - logAuditEvent(true, "createSnapshot", snapshotRoot, rootPath.toString(), - null); + logAuditEvent(true, "createSnapshot", snapshotRoot, snapshotPath, null); } + return snapshotPath; } /** @@ -5833,7 +5835,7 @@ public void createSnapshot(String snapshotRoot, String snapshotName) * @throws SafeModeException * @throws IOException */ - public void renameSnapshot(String path, String snapshotOldName, + void renameSnapshot(String path, String snapshotOldName, String snapshotNewName) throws SafeModeException, IOException { final FSPermissionChecker pc = getPermissionChecker(); writeLock(); @@ -5844,8 +5846,7 @@ public void renameSnapshot(String path, String snapshotOldName, safeMode); } checkOwner(pc, path); - // TODO: check if the new name is valid. May also need this for - // creationSnapshot + dir.verifyMaxComponentLength(snapshotNewName, path, 0); snapshotManager.renameSnapshot(path, snapshotOldName, snapshotNewName); getEditLog().logRenameSnapshot(path, snapshotOldName, snapshotNewName); @@ -5905,7 +5906,7 @@ public SnapshottableDirectoryStatus[] getSnapshottableDirListing() * and labeled as M/-/+/R respectively. * @throws IOException */ - public SnapshotDiffReport getSnapshotDiffReport(String path, + SnapshotDiffReport getSnapshotDiffReport(String path, String fromSnapshot, String toSnapshot) throws IOException { SnapshotDiffInfo diffs = null; readLock(); @@ -5931,7 +5932,7 @@ public SnapshotDiffReport getSnapshotDiffReport(String path, * @throws SafeModeException * @throws IOException */ - public void deleteSnapshot(String snapshotRoot, String snapshotName) + void deleteSnapshot(String snapshotRoot, String snapshotName) throws SafeModeException, IOException { final FSPermissionChecker pc = getPermissionChecker(); writeLock(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java index c69a7cd09ba..f209699a17b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java @@ -1093,14 +1093,14 @@ public DataEncryptionKey getDataEncryptionKey() throws IOException { } @Override - public void createSnapshot(String snapshotRoot, String snapshotName) + public String createSnapshot(String snapshotRoot, String snapshotName) throws IOException { if (!checkPathLength(snapshotRoot)) { throw new IOException("createSnapshot: Pathname too long. Limit " + MAX_PATH_LENGTH + " characters, " + MAX_PATH_DEPTH + " levels."); } metrics.incrCreateSnapshotOps(); - namesystem.createSnapshot(snapshotRoot, snapshotName); + return namesystem.createSnapshot(snapshotRoot, snapshotName); } @Override @@ -1127,6 +1127,9 @@ public void disallowSnapshot(String snapshot) throws IOException { @Override public void renameSnapshot(String snapshotRoot, String snapshotOldName, String snapshotNewName) throws IOException { + if (snapshotNewName == null || snapshotNewName.isEmpty()) { + throw new IOException("The new snapshot name is null or empty."); + } metrics.incrRenameSnapshotOps(); namesystem.renameSnapshot(snapshotRoot, snapshotOldName, snapshotNewName); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java index 9dd4e3d6b75..671790ee47f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/INodeDirectorySnapshottable.java @@ -288,7 +288,7 @@ Snapshot addSnapshot(int id, String name) final int i = searchSnapshot(nameBytes); if (i >= 0) { throw new SnapshotException("Failed to add snapshot: there is already a " - + "snapshot with the same name \"" + name + "\"."); + + "snapshot with the same name \"" + Snapshot.getSnapshotName(s) + "\"."); } final DirectoryDiff d = getDiffs().addDiff(s, this); 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 23a82bce10e..07b05bf25ad 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 @@ -20,7 +20,9 @@ import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; +import java.text.SimpleDateFormat; import java.util.Comparator; +import java.util.Date; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.fs.Path; @@ -37,6 +39,30 @@ public class Snapshot implements Comparable { public static final int INVALID_ID = -1; + /** + * The pattern for generating the default snapshot name. + * E.g. s20130412-151029.033 + */ + private static final String DEFAULT_SNAPSHOT_NAME_PATTERN = "'s'yyyyMMdd-HHmmss.SSS"; + + public static String generateDefaultSnapshotName() { + return new SimpleDateFormat(DEFAULT_SNAPSHOT_NAME_PATTERN).format(new Date()); + } + + static String getSnapshotPath(String snapshottableDir, String snapshotName) { + return new Path(snapshottableDir, HdfsConstants.DOT_SNAPSHOT_DIR + + Path.SEPARATOR + snapshotName).toString(); + } + + /** + * Get the name of the given snapshot. + * @param s The given snapshot. + * @return The name of the snapshot, or an empty string if {@code s} is null + */ + static String getSnapshotName(Snapshot s) { + return s != null ? s.getRoot().getLocalName() : ""; + } + /** * Compare snapshot IDs. Null indicates the current status thus is greater * than non-null snapshots. @@ -78,15 +104,6 @@ public static Snapshot findLatestSnapshot(INode inode, Snapshot anchor) { } return latest; } - - /** - * Get the name of the given snapshot. - * @param s The given snapshot. - * @return The name of the snapshot, or an empty string if {@code s} is null - */ - public static String getSnapshotName(Snapshot s) { - return s != null ? s.getRoot().getLocalName() : ""; - } /** The root directory of the snapshot. */ public class Root extends INodeDirectory { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java index 5b36115f0e0..f69f8f42247 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java @@ -127,19 +127,19 @@ public void resetSnapshottable(final String path * snapshot with the given name for the directory, and/or 3) * snapshot number exceeds quota */ - public void createSnapshot(final String path, final String snapshotName + public String createSnapshot(final String path, String snapshotName ) throws IOException { // Find the source root directory path where the snapshot is taken. final INodesInPath i = fsdir.getINodesInPath4Write(path); final INodeDirectorySnapshottable srcRoot = INodeDirectorySnapshottable.valueOf(i.getLastINode(), path); - fsdir.verifyMaxComponentLength(snapshotName, path, 0); srcRoot.addSnapshot(snapshotCounter, snapshotName); //create success, update id snapshotCounter++; numSnapshots.getAndIncrement(); + return Snapshot.getSnapshotPath(path, snapshotName); } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/ClientNamenodeProtocol.proto b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/ClientNamenodeProtocol.proto index 4fec7142dad..3044cbbab30 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/ClientNamenodeProtocol.proto +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/ClientNamenodeProtocol.proto @@ -450,10 +450,11 @@ message GetDataEncryptionKeyResponseProto { message CreateSnapshotRequestProto { required string snapshotRoot = 1; - required string snapshotName = 2; + optional string snapshotName = 2; } -message CreateSnapshotResponseProto { // void response +message CreateSnapshotResponseProto { + required string snapshotPath = 1; } message RenameSnapshotRequestProto { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestNestedSnapshots.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestNestedSnapshots.java index 571527974ce..14ce1f4fad5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestNestedSnapshots.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestNestedSnapshots.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.Random; +import java.util.regex.Pattern; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; @@ -178,7 +179,19 @@ public void testSnapshotWithQuota() throws Exception { final Path foo = new Path(dir, "foo"); final Path f1 = new Path(foo, "f1"); DFSTestUtil.createFile(hdfs, f1, BLOCKSIZE, REPLICATION, SEED); - hdfs.createSnapshot(dir, "s0"); + { + //create a snapshot with default snapshot name + final Path snapshotPath = hdfs.createSnapshot(dir); + + //check snapshot path and the default snapshot name + final String snapshotName = snapshotPath.getName(); + Assert.assertTrue("snapshotName=" + snapshotName, Pattern.matches( + "s\\d\\d\\d\\d\\d\\d\\d\\d-\\d\\d\\d\\d\\d\\d\\.\\d\\d\\d", + snapshotName)); + final Path parent = snapshotPath.getParent(); + Assert.assertEquals(HdfsConstants.DOT_SNAPSHOT_DIR, parent.getName()); + Assert.assertEquals(dir, parent.getParent()); + } final Path f2 = new Path(foo, "f2"); DFSTestUtil.createFile(hdfs, f2, BLOCKSIZE, REPLICATION, SEED); @@ -193,7 +206,7 @@ public void testSnapshotWithQuota() throws Exception { try { // createSnapshot should fail with quota - hdfs.createSnapshot(dir, "s1"); + hdfs.createSnapshot(dir); Assert.fail(); } catch(RemoteException re) { final IOException ioe = re.unwrapRemoteException();