Apache9 commented on a change in pull request #3851:
URL: https://github.com/apache/hbase/pull/3851#discussion_r753205297
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -2628,10 +2628,10 @@ public long restoreSnapshot(final SnapshotDescription
snapshotDesc,
return MasterProcedureUtil.submitProcedure(
new MasterProcedureUtil.NonceProcedureRunnable(this, nonceGroup,
nonce) {
- @Override
- protected void run() throws IOException {
- setProcId(
- getSnapshotManager().restoreOrCloneSnapshot(snapshotDesc,
getNonceKey(), restoreAcl));
+ @Override protected void run() throws IOException {
Review comment:
Better update the formatter config of your IDE? The annotation should be
on a seprated line...
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
##########
@@ -854,15 +860,75 @@ public long restoreOrCloneSnapshot(final
SnapshotDescription reqSnapshot, final
// Execute the restore/clone operation
long procId;
if (master.getTableDescriptors().exists(tableName)) {
- procId = restoreSnapshot(reqSnapshot, tableName, snapshot,
snapshotTableDesc, nonceKey,
- restoreAcl);
+ procId =
+ restoreSnapshot(reqSnapshot, tableName, snapshot, snapshotTableDesc,
nonceKey, restoreAcl);
} else {
procId =
- cloneSnapshot(reqSnapshot, tableName, snapshot, snapshotTableDesc,
nonceKey, restoreAcl);
+ cloneSnapshot(reqSnapshot, tableName, snapshot, snapshotTableDesc,
nonceKey, restoreAcl,
+ customSFT);
}
return procId;
}
+ // Makes sure restoring a snapshot does not break the current SFT setup
+ // follows StoreUtils.createStoreConfiguration
+ static void checkSFTCompatibility(TableDescriptor currentTableDesc,
+ TableDescriptor snapshotTableDesc, Configuration baseConf) throws
RestoreSnapshotException {
+ //have to compare TableDescriptor.values first
+ String tableDefault =
checkIncompatibleConfig(currentTableDesc.getValue(StoreFileTrackerFactory.TRACKER_IMPL),
+ snapshotTableDesc.getValue(StoreFileTrackerFactory.TRACKER_IMPL),
+ baseConf.get(StoreFileTrackerFactory.TRACKER_IMPL,
StoreFileTrackerFactory.Trackers.DEFAULT.name()),
+ " the Table " + currentTableDesc.getTableName().getNameAsString());
+
+ // have to check existing CFs
+ for (ColumnFamilyDescriptor cfDesc : currentTableDesc.getColumnFamilies())
{
+ ColumnFamilyDescriptor snapCFDesc =
snapshotTableDesc.getColumnFamily(cfDesc.getName());
+ // if there is no counterpart in the snapshot it will be just deleted so
the config does
+ // not matter
+ if (snapCFDesc != null) {
+ // comparing ColumnFamilyDescriptor.conf next
+ String cfDefault = checkIncompatibleConfig(
+ cfDesc.getConfigurationValue(StoreFileTrackerFactory.TRACKER_IMPL),
+
snapCFDesc.getConfigurationValue(StoreFileTrackerFactory.TRACKER_IMPL),
tableDefault,
+ " the config for column family " + cfDesc.getNameAsString());
+
+ // then ColumnFamilyDescriptor.values
+
checkIncompatibleConfig(cfDesc.getValue(StoreFileTrackerFactory.TRACKER_IMPL),
+ snapCFDesc.getValue(StoreFileTrackerFactory.TRACKER_IMPL), cfDefault,
+ " the metadata of column family " + cfDesc.getNameAsString());
+ }
+ }
+ }
+
+ // check if a config change would change the behavior
+ static String checkIncompatibleConfig(String currentValue, String newValue,
String defaultValue,
+ String errorMessage) throws RestoreSnapshotException {
+ Boolean hasIncompatibility = false;
Review comment:
Why Boolean instead of boolean?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
##########
@@ -854,15 +860,75 @@ public long restoreOrCloneSnapshot(final
SnapshotDescription reqSnapshot, final
// Execute the restore/clone operation
long procId;
if (master.getTableDescriptors().exists(tableName)) {
- procId = restoreSnapshot(reqSnapshot, tableName, snapshot,
snapshotTableDesc, nonceKey,
- restoreAcl);
+ procId =
+ restoreSnapshot(reqSnapshot, tableName, snapshot, snapshotTableDesc,
nonceKey, restoreAcl);
} else {
procId =
- cloneSnapshot(reqSnapshot, tableName, snapshot, snapshotTableDesc,
nonceKey, restoreAcl);
+ cloneSnapshot(reqSnapshot, tableName, snapshot, snapshotTableDesc,
nonceKey, restoreAcl,
+ customSFT);
}
return procId;
}
+ // Makes sure restoring a snapshot does not break the current SFT setup
+ // follows StoreUtils.createStoreConfiguration
+ static void checkSFTCompatibility(TableDescriptor currentTableDesc,
+ TableDescriptor snapshotTableDesc, Configuration baseConf) throws
RestoreSnapshotException {
+ //have to compare TableDescriptor.values first
+ String tableDefault =
checkIncompatibleConfig(currentTableDesc.getValue(StoreFileTrackerFactory.TRACKER_IMPL),
+ snapshotTableDesc.getValue(StoreFileTrackerFactory.TRACKER_IMPL),
+ baseConf.get(StoreFileTrackerFactory.TRACKER_IMPL,
StoreFileTrackerFactory.Trackers.DEFAULT.name()),
+ " the Table " + currentTableDesc.getTableName().getNameAsString());
+
+ // have to check existing CFs
+ for (ColumnFamilyDescriptor cfDesc : currentTableDesc.getColumnFamilies())
{
+ ColumnFamilyDescriptor snapCFDesc =
snapshotTableDesc.getColumnFamily(cfDesc.getName());
+ // if there is no counterpart in the snapshot it will be just deleted so
the config does
+ // not matter
+ if (snapCFDesc != null) {
+ // comparing ColumnFamilyDescriptor.conf next
+ String cfDefault = checkIncompatibleConfig(
+ cfDesc.getConfigurationValue(StoreFileTrackerFactory.TRACKER_IMPL),
+
snapCFDesc.getConfigurationValue(StoreFileTrackerFactory.TRACKER_IMPL),
tableDefault,
+ " the config for column family " + cfDesc.getNameAsString());
+
+ // then ColumnFamilyDescriptor.values
+
checkIncompatibleConfig(cfDesc.getValue(StoreFileTrackerFactory.TRACKER_IMPL),
+ snapCFDesc.getValue(StoreFileTrackerFactory.TRACKER_IMPL), cfDefault,
+ " the metadata of column family " + cfDesc.getNameAsString());
+ }
+ }
+ }
+
+ // check if a config change would change the behavior
+ static String checkIncompatibleConfig(String currentValue, String newValue,
String defaultValue,
+ String errorMessage) throws RestoreSnapshotException {
+ Boolean hasIncompatibility = false;
+ //if there is no current override and the snapshot has an override that
does not match the
+ //default
+ if (StringUtils.isEmpty(currentValue) && !StringUtils.isEmpty(newValue) &&
Review comment:
I prefer we write the code like this:
```
if (a != null) {
if (b != null) {
blabla
} else {
blabla
}
} else {
if (b != null) {
blabla
} else {
blabla
}
}
```
The current logic is a bit hard to understand to me...
--
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]
