Apache9 commented on a change in pull request #3851:
URL: https://github.com/apache/hbase/pull/3851#discussion_r768796706



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
##########
@@ -203,6 +216,42 @@ protected Flow executeFromState(final MasterProcedureEnv 
env, final CloneSnapsho
     return Flow.HAS_MORE_STATE;
   }
 
+  /**
+   * If a StoreFileTracker is specified we strip the TableDescriptor from 
previous SFT config
+   * and set the specified SFT on the table level
+   */
+  private void updateTableDescriptorWithSFT() {
+    if (StringUtils.isEmpty(customSFT)){
+      return;
+    }
+
+    TableDescriptorBuilder builder = 
TableDescriptorBuilder.newBuilder(tableDescriptor);
+    builder.setValue(StoreFileTrackerFactory.TRACKER_IMPL, customSFT);
+    for (ColumnFamilyDescriptor family : tableDescriptor.getColumnFamilies()){
+      ColumnFamilyDescriptorBuilder cfBuilder = 
ColumnFamilyDescriptorBuilder.newBuilder(family);
+      cfBuilder.setConfiguration(StoreFileTrackerFactory.TRACKER_IMPL, null);
+      cfBuilder.setValue(StoreFileTrackerFactory.TRACKER_IMPL, null);
+      builder.modifyColumnFamily(cfBuilder.build());
+    }
+    tableDescriptor = builder.build();
+  }
+
+  private void validateSFT() {
+    if (StringUtils.isEmpty(customSFT)){
+      return;
+    }
+
+    try {
+      Configuration sftConfig = new Configuration();
+      sftConfig.set(StoreFileTrackerFactory.TRACKER_IMPL, customSFT);
+      StoreFileTrackerFactory.getTrackerClass(sftConfig);
+    }
+    catch (RuntimeException e){

Review comment:
       Catching RuntimeException is a bit ugly... What happens if we just throw 
the exception out?

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -92,7 +92,7 @@ static String getStoreFileTrackerName(Class<? extends 
StoreFileTracker> clazz) {
     return name != null ? name.name() : clazz.getName();
   }
 
-  private static Class<? extends StoreFileTracker> 
getTrackerClass(Configuration conf) {
+  public static Class<? extends StoreFileTracker> 
getTrackerClass(Configuration conf) {

Review comment:
       Then let's also move the method here?

##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
##########
@@ -1633,7 +1634,26 @@ void restoreSnapshot(String snapshotName, boolean 
takeFailSafeSnapshot, boolean
    */
   default void cloneSnapshot(String snapshotName, TableName tableName)
       throws IOException, TableExistsException, RestoreSnapshotException {
-    cloneSnapshot(snapshotName, tableName, false);
+    cloneSnapshot(snapshotName, tableName, false, null);
+  }
+
+  /**
+   * Create a new table by cloning the snapshot content.
+   * @param snapshotName name of the snapshot to be cloned
+   * @param tableName name of the table where the snapshot will be restored
+   * @param restoreAcl <code>true</code> to clone acl into newly created table
+   * @param customSFT specify the StoreFileTracker used for the table
+   * @throws IOException if a remote or network exception occurs
+   * @throws TableExistsException if table to be created already exists
+   * @throws RestoreSnapshotException if snapshot failed to be cloned
+   * @throws IllegalArgumentException if the specified table has not a valid 
name
+   */
+  @InterfaceStability.Evolving

Review comment:
       Let's remove this one. It should be stable, what is the real concern 
here? The type of customSFT?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to