hudi-agent commented on code in PR #18182:
URL: https://github.com/apache/hudi/pull/18182#discussion_r3237122320
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -2281,6 +2298,93 @@ public static String
deleteMetadataTablePartition(HoodieTableMetaClient dataMeta
return null;
}
+ /**
+ * Returns true if bucketing is enabled for the metadata table.
+ *
+ * <p>Reads the property from the MDT's own table config so we avoid an
extra disk read of the
+ * data table's hoodie.properties on every file-slice lookup. The property
is persisted on both
+ * the data table and the MDT during MDT initialization (see
+ * {@link #setMetadataTablePartitionBucketing(HoodieTableMetaClient,
HoodieTableMetaClient, boolean)}).
+ *
+ * @param metaClient MetaClient for the metadata table
+ * @return true if bucketing is enabled
+ */
+ private static boolean isBucketingEnabledForMDT(HoodieTableMetaClient
metaClient) {
+ return
metaClient.getTableConfig().isMetadataTablePartitionBucketingEnabled();
+ }
+
+ /**
+ * Returns the list of partitions to list for file slices.
+ * When bucketing is enabled, returns the bucket sub-directories.
+ * When bucketing is disabled, returns the partition itself.
+ *
+ * <p>Note: bucketing is fixed at MDT initialization time (see
+ * {@link
org.apache.hudi.metadata.HoodieBackedTableMetadataWriter#initializeFileGroups}).
A given
+ * MDT is therefore either fully bucketed or fully non-bucketed; mixed-state
file groups under
+ * partition/ alongside partition/<bucket>/ cannot occur. We
deliberately do not also scan the
+ * partition root when bucketing is enabled.
+ *
+ * @param metaClient MetaClient for the metadata table
+ * @param partition The partition path
+ * @param bucketingEnabled Whether bucketing is enabled
+ * @return List of partition paths to list
+ */
+ private static List<String> getPartitionsToList(HoodieTableMetaClient
metaClient, String partition, boolean bucketingEnabled) {
+ List<String> partitionsToList = new ArrayList<>();
+ try {
+ if (bucketingEnabled) {
+ // Find all the buckets in the partition
+ StoragePath partitionPath = new StoragePath(metaClient.getBasePath(),
partition);
+ List<StoragePathInfo> statuses =
metaClient.getStorage().listDirectEntries(partitionPath);
+ for (StoragePathInfo status : statuses) {
+ if (status.isDirectory()) {
+ partitionsToList.add(partition + StoragePath.SEPARATOR +
status.getPath().getName());
+ }
+ }
+ log.info("Listing {} buckets in metadata table partition {}",
partitionsToList.size(), partition);
+ } else {
+ // Only list the partition folder as bucketing is not enabled
+ partitionsToList.add(partition);
+ log.info("Listing non-bucketed metadata table partition {}",
partition);
+ }
+ } catch (IOException e) {
+ throw new HoodieMetadataException("Failed to check for bucketing in
metadata table partition " + partition, e);
+ }
+ return partitionsToList;
+ }
+
+ /**
+ * Set the bucketing of partitions within the metadata table.
+ *
+ * <p>The property is persisted on both the data table (authoritative source
for MDT initialization
+ * decisions) and the MDT (to allow reader code to determine bucketing
without loading data table
+ * properties on every call).
+ *
+ * @param dataMetaClient MetaClient for the dataset
+ * @param metadataMetaClient MetaClient for the MDT (may be null if MDT is
not yet initialized)
+ * @param enabled If true, metadata table is being used for this
dataset, false otherwise
+ */
+ public static HoodieTableMetaClient
setMetadataTablePartitionBucketing(HoodieTableMetaClient dataMetaClient,
+
HoodieTableMetaClient metadataMetaClient, boolean enabled) {
+ // Only allowed on the main dataset
+
ValidationUtils.checkArgument(!isMetadataTable(dataMetaClient.getBasePath().toString()),
"Bucketing should only be enabled on the main dataset");
+
+ dataMetaClient.getTableConfig().setMetadataTableBucketing(enabled);
+ HoodieTableConfig.update(dataMetaClient.getStorage(),
dataMetaClient.getMetaPath(), dataMetaClient.getTableConfig().getProps());
+ dataMetaClient = HoodieTableMetaClient.reload(dataMetaClient);
+
ValidationUtils.checkState(dataMetaClient.getTableConfig().isMetadataTablePartitionBucketingEnabled()
== enabled,
+ "Metadata table state change should be persisted");
+
+ if (metadataMetaClient != null) {
+ // Also persist on MDT so reader code can determine bucketing from MDT
props alone.
+ metadataMetaClient.getTableConfig().setMetadataTableBucketing(enabled);
+ HoodieTableConfig.update(metadataMetaClient.getStorage(),
metadataMetaClient.getMetaPath(),
metadataMetaClient.getTableConfig().getProps());
Review Comment:
🤖 If the MDT `HoodieTableConfig.update` here fails after the data-table
update on line 2373 succeeded, we end up with data-table props saying
`bucketing=true` and MDT props still saying `bucketing=false`. The retry guard
in `initializeFileGroups` (`bucketingEnabled &&
!dataMetaClient.getTableConfig().isMetadataTablePartitionBucketingEnabled()`)
then evaluates to false on the next attempt, so this call is never re-issued —
leaving the MDT permanently stale. Writers (which check data-table config) will
keep writing into bucket subdirs, while readers (which now check MDT config via
`isBucketingEnabledForMDT`) will scan the partition root and miss the file
groups. Could you swap the order (update MDT first, then data table) so a
partial failure leaves the data table un-flipped and the next retry naturally
re-attempts both?
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
