nsivabalan commented on code in PR #19045:
URL: https://github.com/apache/hudi/pull/19045#discussion_r3498232573
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -1212,9 +1222,110 @@ private void initializeFileGroups(HoodieTableMetaClient
dataMetaClient, Metadata
writer.appendBlock(block);
}
} catch (InterruptedException e) {
- throw new HoodieException(String.format("Failed to created fileGroup
%s for partition %s", fileGroupFileId, relativePartitionPath), e);
+ throw new HoodieException(String.format("Failed to created fileGroup
%s for partition %s", fileGroupFileId, relativePath), e);
}
- }, fileGroupFileIds.size());
+ }, fileGroupIdAndPath.size());
+
+ // For non-flat layouts, write .hoodie_partition_metadata at the logical
partition root now,
+ // before the HoodieAppendHandle path (which would otherwise create one
inside the bucket
+ // sub-dir). The AppendHandle skips marker creation at layout sub-paths via
+ // HoodieAppendHandle#isMDTLayoutSubPath, so this is the sole marker write
under bucketing.
+ // For the flat default we leave marker creation to the AppendHandle and
write nothing here —
+ // existing tables keep bit-identical behavior.
+ if (!FlatMDTLayout.LAYOUT_ID.equals(layout.getLayoutId())) {
+ for (String markerPath :
layout.getPartitionMarkerPaths(relativePartitionPath, fileGroupCount)) {
+ HoodiePartitionMetadata marker = new HoodiePartitionMetadata(
+ dataMetaClient.getStorage(),
+ instantTime,
+ new StoragePath(metadataWriteConfig.getBasePath()),
+ FSUtils.constructAbsolutePath(metadataWriteConfig.getBasePath(),
markerPath),
+ Option.empty());
+ marker.trySave();
+ }
+ }
+
+ // Persist layout state for this partition (skipped entirely for the flat
default so existing
+ // tables get the identical on-disk and properties layout as before). When
a non-flat layout is
+ // in use, readers consult this property to enumerate physical sub-paths
without an FS listing.
+ if (metadataMetaClient != null &&
!FlatMDTLayout.LAYOUT_ID.equals(layout.getLayoutId())) {
+ maybePersistLayoutOnMDTInit(layout);
+
metadataMetaClient.getTableConfig().addMetadataLayoutPartitionFileGroupCounts(
+ metadataMetaClient, Collections.singletonMap(relativePartitionPath,
fileGroupCount));
Review Comment:
Added in `8a7c60f5c54f`:
`TestHoodieTableConfig.testMetadataLayoutFileGroupCountsRoundTrip` initializes
one MDT partition's count, then appends a later partition's count and verifies
both entries persist together. Companion tests
`testMetadataLayoutFileGroupCountsIdempotentReStamp` and
`testMetadataLayoutFileGroupCountsRejectsConflict` cover the no-op same-count
re-stamp and the immutability rejection (the latter addresses your other
comment about validation).
##########
hudi-common/src/main/java/org/apache/hudi/metadata/FileIdInfo.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.metadata;
+
+import org.apache.hudi.common.util.Option;
+
+import java.io.Serializable;
+
+/**
+ * Parsed representation of an MDT fileId, produced by
+ * {@link HoodieMetadataTableLayout#parseFileId}.
Review Comment:
Done in `92fe6e1972f6` — `FileIdInfo` javadoc now has four concrete examples
(non-partitioned RLI, files, expression index, partitioned RLI) showing fileId
→ `FileIdInfo` translation.
##########
hudi-common/src/main/java/org/apache/hudi/metadata/FileSystemBackedTableMetadata.java:
##########
@@ -80,6 +86,13 @@ public FileSystemBackedTableMetadata(HoodieEngineContext
engineContext, HoodieTa
this.tableName = tableConfig.getTableName();
this.hiveStylePartitioningEnabled =
Boolean.parseBoolean(tableConfig.getHiveStylePartitioningEnable());
this.urlEncodePartitioningEnabled =
Boolean.parseBoolean(tableConfig.getUrlEncodePartitioning());
+ if (HoodieTableMetadata.isMetadataTable(datasetBasePath)) {
+ this.mdtLayout = HoodieMetadataTableLayouts.load(tableConfig);
+ this.mdtLayoutPartitionFileGroupCounts =
tableConfig.getMetadataLayoutPartitionFileGroupCounts();
+ } else {
+ this.mdtLayout = null;
Review Comment:
Done in `54a4741a1785` — both constructors of
`FileSystemBackedTableMetadata` collapsed the `if/else` MDT-field init to a
single `boolean isMdt` + ternary, removing the duplicated `else` branches.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -1212,9 +1222,110 @@ private void initializeFileGroups(HoodieTableMetaClient
dataMetaClient, Metadata
writer.appendBlock(block);
}
} catch (InterruptedException e) {
- throw new HoodieException(String.format("Failed to created fileGroup
%s for partition %s", fileGroupFileId, relativePartitionPath), e);
+ throw new HoodieException(String.format("Failed to created fileGroup
%s for partition %s", fileGroupFileId, relativePath), e);
}
- }, fileGroupFileIds.size());
+ }, fileGroupIdAndPath.size());
+
+ // For non-flat layouts, write .hoodie_partition_metadata at the logical
partition root now,
+ // before the HoodieAppendHandle path (which would otherwise create one
inside the bucket
+ // sub-dir). The AppendHandle skips marker creation at layout sub-paths via
+ // HoodieAppendHandle#isMDTLayoutSubPath, so this is the sole marker write
under bucketing.
+ // For the flat default we leave marker creation to the AppendHandle and
write nothing here —
+ // existing tables keep bit-identical behavior.
+ if (!FlatMDTLayout.LAYOUT_ID.equals(layout.getLayoutId())) {
+ for (String markerPath :
layout.getPartitionMarkerPaths(relativePartitionPath, fileGroupCount)) {
+ HoodiePartitionMetadata marker = new HoodiePartitionMetadata(
+ dataMetaClient.getStorage(),
+ instantTime,
+ new StoragePath(metadataWriteConfig.getBasePath()),
+ FSUtils.constructAbsolutePath(metadataWriteConfig.getBasePath(),
markerPath),
+ Option.empty());
+ marker.trySave();
+ }
+ }
+
+ // Persist layout state for this partition (skipped entirely for the flat
default so existing
+ // tables get the identical on-disk and properties layout as before). When
a non-flat layout is
+ // in use, readers consult this property to enumerate physical sub-paths
without an FS listing.
+ if (metadataMetaClient != null &&
!FlatMDTLayout.LAYOUT_ID.equals(layout.getLayoutId())) {
+ maybePersistLayoutOnMDTInit(layout);
+
metadataMetaClient.getTableConfig().addMetadataLayoutPartitionFileGroupCounts(
Review Comment:
Done in `8a7c60f5c54f` —
`HoodieTableConfig.addMetadataLayoutPartitionFileGroupCounts` now rejects any
attempt to overwrite an existing partition's file-group count with a different
value (throws `HoodieMetadataException`). New partitions can be appended
freely; re-stamping the same count is still a no-op. Covered by
`TestHoodieTableConfig.testMetadataLayoutFileGroupCountsRejectsConflict` +
idempotent re-stamp test.
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1518,27 +1518,52 @@ private static List<FileSlice>
getPartitionFileSlices(HoodieTableMetaClient meta
HoodieTableFileSystemView fsView = null;
try {
fsView = fileSystemView.orElseGet(() ->
getFileSystemViewForMetadataTable(metaClient));
- Stream<FileSlice> fileSliceStream;
- if (mergeFileSlices) {
- if
(metaClient.getActiveTimeline().filterCompletedInstants().lastInstant().isPresent())
{
- fileSliceStream = fsView.getLatestMergedFileSlicesBeforeOrOn(
- // including pending compaction instant as the last instant so
that the finished delta commits
- // that start earlier than the compaction can be queried.
- partition,
metaClient.getActiveTimeline().filterCompletedAndCompactionInstants().lastInstant().get().requestedTime());
+ List<String> physicalPartitions = resolvePhysicalPartitions(metaClient,
partition);
+ List<FileSlice> all = new ArrayList<>();
+ boolean any = false;
+ for (String physical : physicalPartitions) {
Review Comment:
Done in `92fe6e1972f6` — both timeline lookups
(`filterCompletedInstants().lastInstant()` and
`filterCompletedAndCompactionInstants().lastInstant().get().requestedTime()`)
hoisted out of the per-physical-partition loop into local variables before the
loop runs.
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1518,27 +1518,52 @@ private static List<FileSlice>
getPartitionFileSlices(HoodieTableMetaClient meta
HoodieTableFileSystemView fsView = null;
try {
fsView = fileSystemView.orElseGet(() ->
getFileSystemViewForMetadataTable(metaClient));
- Stream<FileSlice> fileSliceStream;
- if (mergeFileSlices) {
- if
(metaClient.getActiveTimeline().filterCompletedInstants().lastInstant().isPresent())
{
- fileSliceStream = fsView.getLatestMergedFileSlicesBeforeOrOn(
- // including pending compaction instant as the last instant so
that the finished delta commits
- // that start earlier than the compaction can be queried.
- partition,
metaClient.getActiveTimeline().filterCompletedAndCompactionInstants().lastInstant().get().requestedTime());
+ List<String> physicalPartitions = resolvePhysicalPartitions(metaClient,
partition);
+ List<FileSlice> all = new ArrayList<>();
+ boolean any = false;
+ for (String physical : physicalPartitions) {
+ Stream<FileSlice> fileSliceStream;
+ if (mergeFileSlices) {
+ if
(metaClient.getActiveTimeline().filterCompletedInstants().lastInstant().isPresent())
{
+ fileSliceStream = fsView.getLatestMergedFileSlicesBeforeOrOn(
+ // including pending compaction instant as the last instant so
that the finished delta commits
+ // that start earlier than the compaction can be queried.
+ physical,
metaClient.getActiveTimeline().filterCompletedAndCompactionInstants().lastInstant().get().requestedTime());
+ } else {
+ return Collections.emptyList();
+ }
} else {
- return Collections.emptyList();
+ fileSliceStream = fsView.getLatestFileSlices(physical);
}
- } else {
- fileSliceStream = fsView.getLatestFileSlices(partition);
+ fileSliceStream.forEach(all::add);
+ any = true;
+ }
+ if (!any) {
+ return Collections.emptyList();
}
- return
fileSliceStream.sorted(Comparator.comparing(FileSlice::getFileId)).collect(Collectors.toList());
+ all.sort(Comparator.comparing(FileSlice::getFileId));
+ return all;
} finally {
- if (!fileSystemView.isPresent()) {
+ if (!fileSystemView.isPresent() && fsView != null) {
fsView.close();
}
}
}
+ /**
+ * Resolve the physical sub-paths to scan for the given MDT partition under
the configured
+ * layout. Layout-aware: for the flat layout this returns {@code
[partition]}; for sub-directory
+ * bucketing it returns one entry per bucket directory. {@code
fileGroupCount} is sourced from
+ * the MDT's persisted layout state — this method does not perform any
filesystem listing.
+ */
+ private static List<String> resolvePhysicalPartitions(HoodieTableMetaClient
metaClient, String partition) {
Review Comment:
Currently `resolvePhysicalPartitions` is called from two sites within
`HoodieTableMetadataUtil` itself (`getPartitionFileSlices` and
`getPartitionLatestFileSlicesIncludingInflight`), so keeping it `private
static` in this file is the minimal-blast-radius placement. If you'd like it
promoted to a `public static` in the same file (it's already in a shared util
class), happy to do that — but moving it to a separate "common class" would be
redundant since `HoodieTableMetadataUtil` is itself the central MDT util class.
Let me know if you want it promoted to public for external callers.
--
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]
