snapshot TTL to be passed as an additional params - based on review c…

…omments
apache · Jul 17, 2019 · b745f8a · b745f8a
1 parent dc3bb17
commit b745f8a
Show file tree

Hide file tree

Showing 10 changed files with 69 additions and 35 deletions.
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
@@ -1381,18 +1381,19 @@ default void snapshot(String snapshotName, TableName tableName, SnapshotType typ
    * {@link org.apache.hadoop.hbase.TableName#isLegalFullyQualifiedTableName(byte[])}.
    * Snapshot can live with ttl seconds.
    *
-   * @param snapshotName name to give the snapshot on the filesystem. Must be unique from all other
-   *                     snapshots stored on the cluster
-   * @param tableName    name of the table to snapshot
-   * @param type         type of snapshot to take
-   * @param ttl          time to live in seconds. After expiry, the snapshot should be deleted
+   * @param snapshotName  name to give the snapshot on the filesystem. Must be unique from all other
+   *                      snapshots stored on the cluster
+   * @param tableName     name of the table to snapshot
+   * @param type          type of snapshot to take
+   * @param snapshotProps snapshot additional properties e.g. TTL
    * @throws IOException               we fail to reach the master
    * @throws SnapshotCreationException if snapshot creation failed
    * @throws IllegalArgumentException  if the snapshot request is formatted incorrectly
    */
-  default void snapshot(String snapshotName, TableName tableName, SnapshotType type, Long ttl)
-          throws IOException, SnapshotCreationException, IllegalArgumentException {
-    snapshot(new SnapshotDescription(snapshotName, tableName, type, ttl));
+  default void snapshot(String snapshotName, TableName tableName, SnapshotType type,
+                        Map<String, Object> snapshotProps) throws IOException,
+      SnapshotCreationException, IllegalArgumentException {
+    snapshot(new SnapshotDescription(snapshotName, tableName, type, snapshotProps));
   }
 
   /**
@@ -1404,17 +1405,18 @@ default void snapshot(String snapshotName, TableName tableName, SnapshotType typ
    * {@link org.apache.hadoop.hbase.TableName#isLegalFullyQualifiedTableName(byte[])}.
    * Snapshot can live with ttl seconds.
    *
-   * @param snapshotName name to give the snapshot on the filesystem. Must be unique from all other
-   *                     snapshots stored on the cluster
-   * @param tableName    name of the table to snapshot
-   * @param ttl          time to live in seconds. After expiry, the snapshot should be deleted
+   * @param snapshotName  name to give the snapshot on the filesystem. Must be unique from all other
+   *                      snapshots stored on the cluster
+   * @param tableName     name of the table to snapshot
+   * @param snapshotProps snapshot additional properties e.g. TTL
    * @throws IOException               we fail to reach the master
    * @throws SnapshotCreationException if snapshot creation failed
    * @throws IllegalArgumentException  if the snapshot request is formatted incorrectly
    */
-  default void snapshot(String snapshotName, TableName tableName, Long ttl)
-          throws IOException, SnapshotCreationException, IllegalArgumentException {
-    snapshot(new SnapshotDescription(snapshotName, tableName, SnapshotType.FLUSH, ttl));
+  default void snapshot(String snapshotName, TableName tableName,
+                        Map<String, Object> snapshotProps) throws IOException,
+      SnapshotCreationException, IllegalArgumentException {
+    snapshot(new SnapshotDescription(snapshotName, tableName, SnapshotType.FLUSH, snapshotProps));
   }
 
   /**

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/SnapshotDescription.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/SnapshotDescription.java
@@ -17,9 +17,13 @@
  */
 package org.apache.hadoop.hbase.client;
 
+import java.util.Map;
+
 import org.apache.hadoop.hbase.TableName;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.org.apache.commons.collections4.MapUtils;
+
 /**
  * The POJO equivalent of HBaseProtos.SnapshotDescription
  */
@@ -49,7 +53,7 @@ public SnapshotDescription(String name, String table) {
   }
 
   public SnapshotDescription(String name, TableName table) {
-    this(name, table, SnapshotType.DISABLED, null, -1, -1, -1);
+    this(name, table, SnapshotType.DISABLED, null, -1, -1, null);
   }
 
   /**
@@ -64,7 +68,7 @@ public SnapshotDescription(String name, String table, SnapshotType type) {
   }
 
   public SnapshotDescription(String name, TableName table, SnapshotType type) {
-    this(name, table, type, null, -1, -1, -1);
+    this(name, table, type, null, -1, -1, null);
   }
 
   /**
@@ -79,35 +83,58 @@ public SnapshotDescription(String name, String table, SnapshotType type, String
   }
 
   public SnapshotDescription(String name, TableName table, SnapshotType type, String owner) {
-    this(name, table, type, owner, -1, -1, -1);
+    this(name, table, type, owner, -1, -1, null);
   }
 
   /**
-   * @see #SnapshotDescription(String, TableName, SnapshotType, String, long, long, int)
+   * @see #SnapshotDescription(String, TableName, SnapshotType, String, long, int, Map)
    * @see <a href="https://issues.apache.org/jira/browse/HBASE-16892">HBASE-16892</a>
    * @deprecated since 2.0.0 and will be removed in 3.0.0. Use the version with the TableName
    *   instance instead.
    */
   @Deprecated
   public SnapshotDescription(String name, String table, SnapshotType type, String owner,
       long creationTime, int version) {
-    this(name, TableName.valueOf(table), type, owner, creationTime, -1, version);
+    this(name, TableName.valueOf(table), type, owner, creationTime, version, null);
   }
 
+  /**
+   * SnapshotDescription Parameterized Constructor
+   *
+   * @param name          Name of the snapshot
+   * @param table         TableName associated with the snapshot
+   * @param type          Type of the snapshot - enum SnapshotType
+   * @param owner         Snapshot Owner
+   * @param creationTime  Creation time for Snapshot
+   * @param version       Snapshot Version
+   * @param snapshotProps Additional properties for snapshot e.g. TTL
+   */
   public SnapshotDescription(String name, TableName table, SnapshotType type, String owner,
-                             long creationTime, long ttl, int version) {
+                             long creationTime, int version, Map<String, Object> snapshotProps) {
     this.name = name;
     this.table = table;
     this.snapShotType = type;
     this.owner = owner;
     this.creationTime = creationTime;
-    this.ttl = ttl;
+    this.ttl = getTtlFromSnapshotProps(snapshotProps);
     this.version = version;
   }
 
+  private long getTtlFromSnapshotProps(Map<String, Object> snapshotProps) {
+    return MapUtils.getLongValue(snapshotProps, "TTL", -1);
+  }
+
+  /**
+   * SnapshotDescription Parameterized Constructor
+   *
+   * @param snapshotName  Name of the snapshot
+   * @param tableName     TableName associated with the snapshot
+   * @param type          Type of the snapshot - enum SnapshotType
+   * @param snapshotProps Additional properties for snapshot e.g. TTL
+   */
   public SnapshotDescription(String snapshotName, TableName tableName, SnapshotType type,
-                             Long ttl) {
-    this(snapshotName, tableName, type, null, -1, ttl, -1);
+                             Map<String, Object> snapshotProps) {
+    this(snapshotName, tableName, type, null, -1, -1, snapshotProps);
   }
 
   public String getName() {

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
@@ -29,6 +29,7 @@
 import java.security.PrivilegedAction;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -2952,10 +2953,12 @@ public static SnapshotType createSnapshotType(SnapshotProtos.SnapshotDescription
    */
   public static SnapshotDescription
       createSnapshotDesc(SnapshotProtos.SnapshotDescription snapshotDesc) {
+    final Map<String, Object> snapshotProps = new HashMap<>();
+    snapshotProps.put("TTL", snapshotDesc.getTtl());
     return new SnapshotDescription(snapshotDesc.getName(),
             snapshotDesc.hasTable() ? TableName.valueOf(snapshotDesc.getTable()) : null,
             createSnapshotType(snapshotDesc.getType()), snapshotDesc.getOwner(),
-            snapshotDesc.getCreationTime(), snapshotDesc.getTtl(), snapshotDesc.getVersion());
+            snapshotDesc.getCreationTime(), snapshotDesc.getVersion(), snapshotProps);
   }
 
   public static RegionLoadStats createRegionLoadStats(ClientProtos.RegionLoadStats stats) {

diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
@@ -1449,7 +1449,7 @@ public enum OperationStatusCode {
   public static final long DEFAULT_SNAPSHOT_TTL = 0;
 
   // User defined Default TTL config key
-  public static final String DEFAULT_SNAPSHOT_TTL_CONFIG_KEY = "hbase.master.snapshot.ttl.default";
+  public static final String DEFAULT_SNAPSHOT_TTL_CONFIG_KEY = "hbase.master.snapshot.ttl";
 
   public static final String SNAPSHOT_CLEANER_DISABLE = "hbase.master.cleaner.snapshot.disable";
 

diff --git a/hbase-common/src/main/resources/hbase-default.xml b/hbase-common/src/main/resources/hbase-default.xml
@@ -1875,7 +1875,7 @@ possible configurations would overwhelm and obscure the important.
     </description>
   </property>
   <property>
-    <name>hbase.master.snapshot.ttl.default</name>
+    <name>hbase.master.snapshot.ttl</name>
     <value>0</value>
     <description>
       Default Snapshot TTL to be considered when the user does not specify TTL while

diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotFromClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotFromClient.java
@@ -219,7 +219,7 @@ public void testOfflineTableSnapshot() throws Exception {
     String snapshot = SNAPSHOT_NAME;
 
     admin.snapshot(new SnapshotDescription(SNAPSHOT_NAME, TABLE_NAME,
-            SnapshotType.DISABLED, null, -1, 1000, SnapshotManifestV1.DESCRIPTOR_VERSION));
+        SnapshotType.DISABLED, null, -1, SnapshotManifestV1.DESCRIPTOR_VERSION, null));
     LOG.debug("Snapshot completed.");
 
     // make sure we have the snapshot

diff --git a/...e-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotTemporaryDirectory.java b/...e-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotTemporaryDirectory.java
@@ -472,7 +472,7 @@ private void takeSnapshot(TableName tableName, String snapshotName, boolean disa
       throws IOException {
     SnapshotType type = disabled ? SnapshotType.DISABLED : SnapshotType.FLUSH;
     SnapshotDescription desc = new SnapshotDescription(snapshotName, tableName, type, null, -1,
-            -1, manifestVersion);
+        manifestVersion, null);
     admin.snapshot(desc);
   }
 }
diff --git a/hbase-shell/src/main/ruby/hbase/admin.rb b/hbase-shell/src/main/ruby/hbase/admin.rb
@@ -1110,11 +1110,13 @@ def snapshot(table, snapshot_name, *args)
         args.each do |arg|
           ttl = arg[TTL]
           ttl = ttl ? ttl.to_java(:long) : -1
+          snapshot_props = java.util.HashMap.new
+          snapshot_props.put("TTL", ttl)
           if arg[SKIP_FLUSH] == true
             @admin.snapshot(snapshot_name, table_name,
-                            org.apache.hadoop.hbase.client.SnapshotType::SKIPFLUSH, ttl)
+                            org.apache.hadoop.hbase.client.SnapshotType::SKIPFLUSH, snapshot_props)
           else
-            @admin.snapshot(snapshot_name, table_name, ttl)
+            @admin.snapshot(snapshot_name, table_name, snapshot_props)
           end
         end
       end

diff --git a/src/main/asciidoc/_chapters/hbase-default.adoc b/src/main/asciidoc/_chapters/hbase-default.adoc
@@ -2147,8 +2147,8 @@ The percent of region server RPC threads failed to abort RS.
 `1800000`
 
 
-[[hbase.master.snapshot.ttl.default]]
-*`hbase.master.snapshot.ttl.default`*::
+[[hbase.master.snapshot.ttl]]
+*`hbase.master.snapshot.ttl`*::
 +
 .Description
 

diff --git a/src/main/asciidoc/_chapters/ops_mgt.adoc b/src/main/asciidoc/_chapters/ops_mgt.adoc
@@ -2767,13 +2767,13 @@ and hence, the snapshot is supposed to be cleaned up after 24 hours
 .Default Snapshot TTL:
 
 - FOREVER by default
-- User specified Default TTL with config `hbase.master.snapshot.ttl.default`
+- User specified Default TTL with config `hbase.master.snapshot.ttl`
 
 
 While creating a Snapshot, if TTL in seconds is not specified, by default the snapshot
 would not be deleted automatically. i.e. it would be retained forever until it is
 manually deleted. However, the user can update this default TTL behavior by
-providing default TTL in sec for key: `hbase.master.snapshot.ttl.default`.
+providing default TTL in sec for key: `hbase.master.snapshot.ttl`.
 Value 0 for this config indicates TTL: FOREVER