yihua commented on code in PR #13644:
URL: https://github.com/apache/hudi/pull/13644#discussion_r2241033576
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -275,4 +280,25 @@ protected Pair<Map<ConfigProperty, String>,
List<ConfigProperty>> downgrade(Hood
throw new HoodieUpgradeDowngradeException(fromVersion.versionCode(),
toVersion.versionCode(), false);
}
}
+
+ /**
+ * Handles the case when auto-upgrade is disabled for upgrade operations.
+ * Sets compatible writer version and determines if upgrade logic should be
skipped.
+ *
+ * @param fromVersion current table version
+ * @param toVersion target table version
+ * @return true if upgrade logic should be skipped, false otherwise
+ */
+ private boolean handleAutoUpgradeDisabled(HoodieTableVersion fromVersion,
HoodieTableVersion toVersion) {
+ if (!config.autoUpgrade() && fromVersion.versionCode() <
toVersion.versionCode()) {
+ // Set appropriate writer version based on upgrade transition to
maintain compatibility
+ if (fromVersion == HoodieTableVersion.SEVEN && toVersion ==
HoodieTableVersion.EIGHT) {
Review Comment:
`fromVersion` and `toVersion` can be two versions apart. So should this
consider more cases, not only the cases where `fromVersion` and `toVersion` are
one version apart?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -130,6 +130,17 @@ public boolean needsUpgrade(HoodieTableVersion
toWriteVersion) {
* @param instantTime current instant time that should not be touched.
*/
public void run(HoodieTableVersion toVersion, String instantTime) {
Review Comment:
Could you also audit the callers of the method and upgrade/downgrade
process? I'm wondering if the `UpgradeDowngrade` should only be called if
upgrade or downgrade is needed; and the caller should check auto upgrade flag.
From what I read from the code:
- `BaseHoodieWriteClient#tryUpgrade` checks whether the table needs to be
upgraded using `hoodie.write.table.version`. Also note that this method takes
care of rollbacks already. So there is no need to do rollback within upgrade
and downgrade handlers?
--
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]
