nsivabalan commented on code in PR #13642:
URL: https://github.com/apache/hudi/pull/13642#discussion_r2240388841


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -144,16 +154,12 @@ public void run(HoodieTableVersion toVersion, String 
instantTime) {
               .run(toVersion, instantTime);
         }
       } catch (Exception e) {
-        LOG.warn("Unable to upgrade or downgrade the metadata table to version 
" + toVersion
-            + ", ignoring the error and continue.", e);
+        throw new HoodieUpgradeDowngradeException("Upgrade/downgrade for the 
Hudi metadata table failed. "
+            + "Please try again. If the failure repeats for metadata table, it 
is recommended to delete "

Review Comment:
   `recommended to disable metadata` 



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -275,4 +281,98 @@ protected Pair<Map<ConfigProperty, String>, 
List<ConfigProperty>> downgrade(Hood
       throw new HoodieUpgradeDowngradeException(fromVersion.versionCode(), 
toVersion.versionCode(), false);
     }
   }
+
+  protected UpgradeHandler getUpgradeHandlerInstance(HoodieTableVersion 
fromVersion, HoodieTableVersion toVersion) {
+    if (fromVersion == HoodieTableVersion.ZERO && toVersion == 
HoodieTableVersion.ONE) {
+      return new ZeroToOneUpgradeHandler();
+    } else if (fromVersion == HoodieTableVersion.ONE && toVersion == 
HoodieTableVersion.TWO) {
+      return new OneToTwoUpgradeHandler();
+    } else if (fromVersion == HoodieTableVersion.TWO && toVersion == 
HoodieTableVersion.THREE) {
+      return new TwoToThreeUpgradeHandler();
+    } else if (fromVersion == HoodieTableVersion.THREE && toVersion == 
HoodieTableVersion.FOUR) {
+      return new ThreeToFourUpgradeHandler();
+    } else if (fromVersion == HoodieTableVersion.FOUR && toVersion == 
HoodieTableVersion.FIVE) {
+      return new FourToFiveUpgradeHandler();
+    } else if (fromVersion == HoodieTableVersion.FIVE && toVersion == 
HoodieTableVersion.SIX) {
+      return new FiveToSixUpgradeHandler();
+    } else if (fromVersion == HoodieTableVersion.SIX && toVersion == 
HoodieTableVersion.SEVEN) {
+      return new SixToSevenUpgradeHandler();
+    } else if (fromVersion == HoodieTableVersion.SEVEN && toVersion == 
HoodieTableVersion.EIGHT) {
+      return new SevenToEightUpgradeHandler();
+    } else if (fromVersion == HoodieTableVersion.EIGHT && toVersion == 
HoodieTableVersion.NINE) {
+      return new EightToNineUpgradeHandler();
+    } else {
+      throw new HoodieUpgradeDowngradeException(fromVersion.versionCode(), 
toVersion.versionCode(), true);
+    }
+  }
+
+  protected DowngradeHandler getDowngradeHandlerInstance(HoodieTableVersion 
fromVersion, HoodieTableVersion toVersion) {
+    if (fromVersion == HoodieTableVersion.ONE && toVersion == 
HoodieTableVersion.ZERO) {
+      return new OneToZeroDowngradeHandler();
+    } else if (fromVersion == HoodieTableVersion.TWO && toVersion == 
HoodieTableVersion.ONE) {
+      return new TwoToOneDowngradeHandler();
+    } else if (fromVersion == HoodieTableVersion.THREE && toVersion == 
HoodieTableVersion.TWO) {
+      return new ThreeToTwoDowngradeHandler();
+    } else if (fromVersion == HoodieTableVersion.FOUR && toVersion == 
HoodieTableVersion.THREE) {
+      return new FourToThreeDowngradeHandler();
+    } else if (fromVersion == HoodieTableVersion.FIVE && toVersion == 
HoodieTableVersion.FOUR) {
+      return new FiveToFourDowngradeHandler();
+    } else if (fromVersion == HoodieTableVersion.SIX && toVersion == 
HoodieTableVersion.FIVE) {
+      return new SixToFiveDowngradeHandler();
+    } else if (fromVersion == HoodieTableVersion.SEVEN && toVersion == 
HoodieTableVersion.SIX) {
+      return new SevenToSixDowngradeHandler();
+    } else if (fromVersion == HoodieTableVersion.EIGHT && toVersion == 
HoodieTableVersion.SEVEN) {
+      return new EightToSevenDowngradeHandler();
+    } else if (fromVersion == HoodieTableVersion.NINE && toVersion == 
HoodieTableVersion.EIGHT) {
+      return new NineToEightDowngradeHandler();
+    } else {
+      throw new HoodieUpgradeDowngradeException(fromVersion.versionCode(), 
toVersion.versionCode(), false);
+    }
+  }
+
+  /**
+   * Checks if any handlers in the upgrade/downgrade path need rollback and 
compaction and performs it once before starting.
+   * This ensures rollback and compaction happens only when needed and only 
once at the very beginning of the process.
+   *
+   * @param fromVersion the current table version
+   * @param toVersion   the target table version
+   */
+  private void rollbackAndCompactIfNeeded(HoodieTableVersion fromVersion, 
HoodieTableVersion toVersion) {
+    // Check if any handlers in the upgrade/downgrade path need rollback and 
compaction
+    boolean needsRollbackAndCompact = false;
+    if (fromVersion.versionCode() < toVersion.versionCode()) {
+      // Check upgrade handlers
+      HoodieTableVersion checkVersion = fromVersion;
+      while (checkVersion.versionCode() < toVersion.versionCode()) {
+        HoodieTableVersion nextVersion = 
HoodieTableVersion.fromVersionCode(checkVersion.versionCode() + 1);
+        UpgradeHandler handler = getUpgradeHandlerInstance(checkVersion, 
nextVersion);

Review Comment:
   do we really need to instantiate upgradeHandler instance here just to 
understand if we need to do rollback and compact. 
   currently, I don't see much intelligence within a certain upgrade/downgrade 
handler based on which we do rollback pending and compact. 
   
   if we are don't see much more logic in there, can we simplify this w/ just a 
set 
   
   UPGRADE_HANDLER_REQUIRING_ROLLBACK_PENDING_COMMITS = {{7 -> 8}, {8 ->9}}
   and just lookup in this set would suffice. 
   CC @yihua 



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -144,16 +154,12 @@ public void run(HoodieTableVersion toVersion, String 
instantTime) {
               .run(toVersion, instantTime);
         }
       } catch (Exception e) {
-        LOG.warn("Unable to upgrade or downgrade the metadata table to version 
" + toVersion
-            + ", ignoring the error and continue.", e);
+        throw new HoodieUpgradeDowngradeException("Upgrade/downgrade for the 
Hudi metadata table failed. "

Review Comment:
   can we write a test for this case. 



-- 
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]

Reply via email to