wombatu-kun commented on code in PR #19426:
URL: https://github.com/apache/hudi/pull/19426#discussion_r3687559500
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncTool.java:
##########
@@ -239,9 +240,9 @@ protected void syncHoodieTable(String tableName, boolean
useRealtimeInputFormat,
log.info("Trying to sync hoodie table {} with base path {} of type {}",
tableName, syncClient.getBasePath(), syncClient.getTableType());
final boolean tableExists = syncClient.tableExists(tableName);
- // if table exists and location of the metastore table doesn't match the
hoodie base path, recreate the table
- if (tableExists &&
!FSUtils.comparePathsWithoutScheme(syncClient.getBasePath(),
syncClient.getTableLocation(tableName))) {
- log.info("basepath is updated for the table {}", tableName);
+ // recreate the table if it exists and either its metastore location no
longer matches the hoodie base path,
+ // or a full recreate was explicitly requested
+ if (tableExists && shouldRecreateTable(tableName)) {
Review Comment:
With hoodie.meta.sync.force.recreate.table on, this branch is taken on every
sync of an existing table, so updateHoodieWriterVersion is never reached and
the recreated table permanently loses the hudi_writer_version property that
createOrReplaceTable does not set. Add an updateHoodieWriterVersion call to
recreateAndSyncHiveTable so the recreate path republishes it.
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncTool.java:
##########
@@ -360,6 +361,22 @@ protected boolean shouldRecreateAndSyncTable() {
return config.getBooleanOrDefault(RECREATE_HIVE_TABLE_ON_ERROR);
}
+ /**
+ * Whether the metastore table (assumed to already exist) should be dropped
and recreated, either because a
+ * full recreate was explicitly requested, or because its stored location no
longer matches the hoodie base path.
+ */
+ private boolean shouldRecreateTable(String tableName) {
Review Comment:
shouldRecreateTable and the pre-existing shouldRecreateAndSyncTable read as
the same thing but are different triggers - this one is the pre-sync check, the
other the on-error fallback. Rename this one to something like
shouldRecreateTableBeforeSync; nit, not a blocker.
##########
hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/HoodieSyncConfig.java:
##########
@@ -182,6 +182,16 @@ public class HoodieSyncConfig extends HoodieConfig {
.withDocumentation("If true, TOUCH partition events will be emitted
during meta sync. "
+ "TOUCH events indicate partitions that exist in both storage and
metastore, no schema or location change, but the partition has received data.");
+ public static final ConfigProperty<Boolean> META_SYNC_FORCE_RECREATE_TABLE =
ConfigProperty
+ .key("hoodie.meta.sync.force.recreate.table")
Review Comment:
This key sits in HoodieSyncConfig but only HiveSyncTool reads it, so
BigQuerySyncTool, AdbSyncTool and DataHubSyncTool will accept it and silently
do nothing. Was the generic meta.sync scope intended, given
RECREATE_HIVE_TABLE_ON_ERROR and RECREATE_GLUE_TABLE_ON_ERROR are both
catalog-scoped and AdbSyncConfig already carries its own
drop_table_before_creation?
--
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]
