nsivabalan commented on code in PR #19045:
URL: https://github.com/apache/hudi/pull/19045#discussion_r3611503934
##########
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:
Yes — added in `8a7c60f5` under `TestHoodieTableConfig`: three tests
exercise the partition→file-group-count map: (1) initial write + later
partition append round-trip, (2) idempotent re-stamp of the same value, (3)
conflict rejection when a later init tries to overwrite an existing partition’s
count with a different value. That third one is the concrete
"already-initialized partition cannot change file-group count" contract.
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataFileIdInfo.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 `92fe6e19`. `HoodieMetadataFileIdInfo` (renamed from `FileIdInfo`
per hudi-agent above) now has fileId-to-`FileIdInfo` examples in the javadoc
covering the four shapes the SPI may parse: non-partitioned RLI, files,
expression index, and partitioned RLI.
##########
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 `54a4741a`. Both `FileSystemBackedTableMetadata` constructors now
use a single boolean lookup + ternary to set `mdtLayout` and
`mdtLayoutPartitionFileGroupCounts`, removing the else branch as suggested.
##########
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 `8a7c60f5`.
`HoodieTableConfig.addMetadataLayoutPartitionFileGroupCounts` now rejects any
attempt to overwrite an existing partition’s count with a different value.
Re-stamping the same count stays idempotent. Intent: the on-disk bucket layout
for an existing MDT partition stays immutable so readers cannot find it
relocated. New MDT partitions can still be appended freely. Test coverage lives
in `TestHoodieTableConfig` (see reply on evolving-count test above).
##########
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 `92fe6e19`. `HoodieTableMetadataUtil.getPartitionFileSlices` now
hoists the timeline lookups (`filterCompletedInstants().lastInstant()` and
`filterCompletedAndCompactionInstants().lastInstant()`) out of the
per-physical-partition loop — they depend only on `metaClient` and stay
invariant across iterations. The post-loop `any` flag will also become an
early-return per hudi-agent’s adjacent nit in a follow-up.
--
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]
