joshelser commented on a change in pull request #3851:
URL: https://github.com/apache/hbase/pull/3851#discussion_r765345147
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
##########
@@ -1633,7 +1633,25 @@ 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 StroreFileTracker used for the table
Review comment:
```suggestion
* @param customSFT specify the StoreFileTracker used for the table
```
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
##########
@@ -1633,7 +1633,25 @@ 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 StroreFileTracker 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
+ */
+ default void cloneSnapshot(String snapshotName, TableName tableName, boolean
restoreAcl,
+ String customSFT)
Review comment:
Admin is public API, so this is a commitment that using a String for SFT
is the path forward. Do we want a String vs. an enum? I'm not positive if our
rules say that we can mark just this one method as private/unstable until we
are sure SFT is ready for all users.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
##########
@@ -203,6 +216,26 @@ 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);
Review comment:
Do we have any ability to validate that this is a valid `customSFT`
prior to modifying the TableDesc? Maybe a call we could make on
`StoreFileTrackerFactory`?
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
##########
@@ -1633,7 +1633,25 @@ 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 StroreFileTracker 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
+ */
+ default void cloneSnapshot(String snapshotName, TableName tableName, boolean
restoreAcl,
+ String customSFT)
Review comment:
Ok, Admin is IA.Public which requires it to be IS.Stable. Rereading the
hbase book, I don't think there's anything stopping us from claiming this one
method is IA.LimitedPrivate/IS.Evolving. Maybe someone else can verify my
thinking.
##########
File path: hbase-shell/src/main/ruby/shell/commands/clone_snapshot.rb
##########
@@ -33,13 +33,17 @@ def help
newly created table.
hbase> clone_snapshot 'snapshotName', 'namespace:tableName',
{RESTORE_ACL=>true}
+
+StoreFileTracker implementation used after restore can be set by the following
command.
+ hbase> clone_snapshot 'snapshotName', 'namespace:tableName',
{CLONE_SFT=>FILE}
Review comment:
should be `{CLONE_SFT=>"FILE"}`?
--
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]
