yihua commented on code in PR #20276:
URL: https://github.com/apache/hudi/pull/20276#discussion_r4365115921


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -1710,11 +1712,31 @@ protected void tryUpgrade(HoodieTableMetaClient 
metaClient, Option<String> insta
       metaClient.reloadTableConfig();
       metaClient.reloadActiveTimeline();
     }
+
+    backfillTableConfigs(metaClient);
+  }
+
+  private void backfillTableConfigs(HoodieTableMetaClient metaClient) {
+    if 
(!HoodieTableConfig.getMissingKeyGeneratorClass(metaClient.getTableConfig()).isPresent())
 {
+      return;
+    }
+    // another writer may have backfilled the table since this meta client 
loaded its config
+    metaClient.reloadTableConfig();
+    Option<String> missingKeyGeneratorClass = 
HoodieTableConfig.getMissingKeyGeneratorClass(metaClient.getTableConfig());
+    if (missingKeyGeneratorClass.isPresent()) {
+      log.info("Backfilling {}={} for table {}", 
HoodieTableConfig.KEY_GENERATOR_CLASS_NAME.key(),
+          missingKeyGeneratorClass.get(), metaClient.getBasePath());
+      Properties updatedProps = new Properties();
+      
updatedProps.setProperty(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME.key(), 
missingKeyGeneratorClass.get());
+      HoodieTableConfig.update(metaClient.getStorage(), 
metaClient.getMetaPath(), updatedProps);

Review Comment:
   Done, the tryUpgrade javadoc now notes that the Flink coordinator calls it 
without the lock.



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -1710,11 +1712,31 @@ protected void tryUpgrade(HoodieTableMetaClient 
metaClient, Option<String> insta
       metaClient.reloadTableConfig();
       metaClient.reloadActiveTimeline();
     }
+
+    backfillTableConfigs(metaClient);

Review Comment:
   Thanks both. This is the same unlocked path the Flink upgrade already takes, 
so I will leave the locking to the follow-up that moves these one-time table 
actions into the coordinator, and I noted the exception in the tryUpgrade 
javadoc.



##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -701,12 +701,30 @@ public static void create(HoodieStorage storage, 
StoragePath metadataFolder, Pro
         hoodieConfig.setValue(POPULATE_META_FIELDS,
             Boolean.toString(metaFieldsMode.toLegacyPopulateMetaFields()));
       }
+      getMissingKeyGeneratorClass(hoodieConfig).ifPresent(
+          keyGeneratorClass -> hoodieConfig.setValue(KEY_GENERATOR_CLASS_NAME, 
keyGeneratorClass));
 
       dropInvalidConfigs(hoodieConfig);
       storeProperties(hoodieConfig.getProps(), outputStream, propertyPath);
     }
   }
 
+  /**
+   * Returns the key generator class that a table below version 8 records only 
through its key generator type.
+   * Hudi 0.x reads only the class, so such a table must carry it as well.
+   */
+  public static Option<String> getMissingKeyGeneratorClass(HoodieConfig 
config) {
+    if (!getTableVersion(config).lesserThan(HoodieTableVersion.EIGHT)

Review Comment:
   Thanks for the suggestion. I will keep it as a single condition, since it 
lists the three cases that need no backfill.



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