[GitHub] [hudi] xushiyan commented on a diff in pull request #6731: [HUDI-4848] Fixing repair deprecated partition tool

[GitBox <gi...@apache.org>]

xushiyan commented on code in PR #6731:
URL: https://github.com/apache/hudi/pull/6731#discussion_r981978986


##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/SparkMain.java:
##########
@@ -456,8 +458,15 @@ public static int renamePartition(JavaSparkContext jsc, String basePath, String
       HoodieTableMetaClient metaClient = HoodieTableMetaClient.builder().setConf(jsc.hadoopConfiguration()).setBasePath(basePath).build();
       Map<String, String> propsMap = getPropsForRewrite(metaClient);
       rewriteRecordsToNewPartition(basePath, newPartition, recordsToRewrite, metaClient, propsMap);
-      // after re-writing, we can safely delete older data.
+      // after re-writing, we can safely delete older partition.
       deleteOlderPartition(basePath, oldPartition, recordsToRewrite, propsMap);
+      // also, we can physically delete the old partition.
+      FileSystem fs = FSUtils.getFs(new Path(basePath), metaClient.getHadoopConf());
+      try {
+        fs.delete(new Path(basePath, oldPartition), true);
+      } catch (IOException e) {
+        LOG.warn("Failed to delete older partition " + basePath);

Review Comment:
   so if this failed we left some dir in the table? better to return 1 and make the operation fail so user would notice it. the warn log can easily go unnoticed. /nit the msg should contain basePath + oldPartition



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org