yihua commented on code in PR #13519:
URL: https://github.com/apache/hudi/pull/13519#discussion_r2263430626
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -275,4 +287,30 @@ protected Pair<Map<ConfigProperty, String>,
List<ConfigProperty>> downgrade(Hood
throw new HoodieUpgradeDowngradeException(fromVersion.versionCode(),
toVersion.versionCode(), false);
}
}
+
+ /**
+ * Class to hold the change set required to update or delete from table
config properties.
+ */
+ static class TableConfigChangeSet {
+ private final Map<ConfigProperty, String> propertiesToUpdate;
+ private final List<ConfigProperty> propertiesToDelete;
+
+ public TableConfigChangeSet() {
+ this.propertiesToUpdate = Collections.emptyMap();
+ this.propertiesToDelete = Collections.EMPTY_LIST;
+ }
+
+ public TableConfigChangeSet(Map<ConfigProperty, String>
propertiesToUpdate, List<ConfigProperty> propertiesToDelete) {
+ this.propertiesToUpdate = propertiesToUpdate;
+ this.propertiesToDelete = propertiesToDelete;
+ }
+
+ public Map<ConfigProperty, String> getPropertiesToUpdate() {
+ return propertiesToUpdate;
+ }
+
+ public List<ConfigProperty> getPropertiesToDelete() {
+ return propertiesToDelete;
+ }
Review Comment:
```suggestion
public Map<ConfigProperty, String> propertiesToUpdate() {
return propertiesToUpdate;
}
public List<ConfigProperty> propertiesToDelete() {
return propertiesToDelete;
}
```
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -164,17 +167,20 @@ public void run(HoodieTableVersion toVersion, String
instantTime) {
// upgrade
while (fromVersion.versionCode() < toVersion.versionCode()) {
HoodieTableVersion nextVersion =
HoodieTableVersion.fromVersionCode(fromVersion.versionCode() + 1);
- tablePropsToAdd.putAll(upgrade(fromVersion, nextVersion, instantTime));
+ UpgradeDowngrade.TableConfigChangeSet tableConfigChangeSet =
+ upgrade(fromVersion, nextVersion, instantTime);
+ tablePropsToAdd.putAll(tableConfigChangeSet.getPropertiesToUpdate());
+
tablePropsToRemove.addAll(tableConfigChangeSet.getPropertiesToDelete());
fromVersion = nextVersion;
}
} else {
// downgrade
isDowngrade = true;
while (fromVersion.versionCode() > toVersion.versionCode()) {
HoodieTableVersion prevVersion =
HoodieTableVersion.fromVersionCode(fromVersion.versionCode() - 1);
- Pair<Map<ConfigProperty, String>, List<ConfigProperty>>
tablePropsToAddAndRemove = downgrade(fromVersion, prevVersion, instantTime);
- tablePropsToAdd.putAll(tablePropsToAddAndRemove.getLeft());
- tablePropsToRemove.addAll(tablePropsToAddAndRemove.getRight());
+ UpgradeDowngrade.TableConfigChangeSet tableConfigChangeSet =
downgrade(fromVersion, prevVersion, instantTime);
+ tablePropsToAdd.putAll(tableConfigChangeSet.getPropertiesToUpdate());
+
tablePropsToRemove.addAll(tableConfigChangeSet.getPropertiesToDelete());
Review Comment:
Given that the upgrade and downgrade between table version 6 and 8 requires
major log format and timeline layout change, should the table configs be
rewritten for every upgrade and downgrade step, i.e., for 6 -> 9 upgrade, it
should rewrite table configs after each of 6 -> 7, 7 -> 8, 8 -> 9 upgrades?
I'm wondering if `tablePropsToAdd` and `tablePropsToRemove` have overlap config
keys after merging multiple changes from multiple upgrades, could that lead to
correctness issue.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -275,4 +287,30 @@ protected Pair<Map<ConfigProperty, String>,
List<ConfigProperty>> downgrade(Hood
throw new HoodieUpgradeDowngradeException(fromVersion.versionCode(),
toVersion.versionCode(), false);
}
}
+
+ /**
+ * Class to hold the change set required to update or delete from table
config properties.
+ */
+ static class TableConfigChangeSet {
+ private final Map<ConfigProperty, String> propertiesToUpdate;
+ private final List<ConfigProperty> propertiesToDelete;
Review Comment:
nit: should this be a `Set`?
--
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]
