joshelser commented on a change in pull request #3851:
URL: https://github.com/apache/hbase/pull/3851#discussion_r768824485
##########
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){
+ throw new UnsupportedOperationException("Specified SFT: " + customSFT +
" was not recognized",
+ e);
Review comment:
```suggestion
} catch (RuntimeException e) {
throw new UnsupportedOperationException("Specified SFT: " + customSFT
+ " was not recognized",
e);
```
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -311,4 +312,40 @@ public static void checkForModifyTable(Configuration conf,
TableDescriptor oldTa
}
}
}
+
+ /**
+ * Makes sure restoring a snapshot does not break the current SFT setup
+ * follows StoreUtils.createStoreConfiguration
+ * @param currentTableDesc Existing Table's TableDescriptor
+ * @param snapshotTableDesc Snapshot's TableDescriptor
+ * @param baseConf Current global configuration
+ * @throws RestoreSnapshotException if restore would break the current SFT
setup
+ */
+ public static void checkForRestoreSnapshot(TableDescriptor currentTableDesc,
Review comment:
nit: maybe `validatePreRestoreSnapshot(..)` to better indicate what
we're doing (validating the state) and when we are doing it (before a
restore_snapshot).
"Validate" is a bit more specific than "check" to me, but I recognize this
is also subjective.
##########
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:
Seems OK to have these public here since `checkForRestoreSnapshot`
method is also used in `SnapshotManager`. Seems overkill to add an interface to
this Factory just for these two ("internal public api") methods, which is what
we'd normally do. For a Factory class, I would expect I could get/create things
as the caller.
--
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]
