voonhous commented on code in PR #19243:
URL: https://github.com/apache/hudi/pull/19243#discussion_r3635395077
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java:
##########
@@ -195,6 +195,14 @@ public final class HoodieMetadataConfig extends
HoodieConfig {
.sinceVersion("0.7.0")
.withDocumentation("Directories matching this regex, will be filtered
out when initializing metadata table from lake storage for the first time.");
+ public static final ConfigProperty<Boolean>
SKIP_ZERO_SIZE_FILES_ON_INITIALIZE = ConfigProperty
+ .key(METADATA_PREFIX + ".skip.zero.size.files.on.initialize")
+ .defaultValue(false)
+ .markAdvanced()
+ .sinceVersion("1.2.0")
+ .withDocumentation("When enabled, zero-size data files encountered while
listing the data table during "
+ + "metadata table initialization are skipped instead of being
recorded in the metadata table.");
Review Comment:
Can we add the operational caveat to the doc? e.g.
> Skipped files remain on storage and are not tracked by the metadata table
or the cleaner; remove them manually. The metadata validator will report them
as inconsistencies.
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java:
##########
@@ -195,6 +195,14 @@ public final class HoodieMetadataConfig extends
HoodieConfig {
.sinceVersion("0.7.0")
.withDocumentation("Directories matching this regex, will be filtered
out when initializing metadata table from lake storage for the first time.");
+ public static final ConfigProperty<Boolean>
SKIP_ZERO_SIZE_FILES_ON_INITIALIZE = ConfigProperty
+ .key(METADATA_PREFIX + ".skip.zero.size.files.on.initialize")
+ .defaultValue(false)
+ .markAdvanced()
+ .sinceVersion("1.2.0")
Review Comment:
`sinceVersion` should be `1.3.0` -- master is on `1.3.0-SNAPSHOT` now.
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -3130,11 +3136,20 @@ public DirectoryInfo(String relativePath,
List<StoragePathInfo> pathInfos, Strin
String dataFileCommitTime =
FSUtils.getCommitTime(pathInfo.getPath().getName());
// Limit the file listings to files which were created by successful
commits before the maxInstant time.
if (!pendingDataInstants.contains(dataFileCommitTime) &&
compareTimestamps(dataFileCommitTime, LESSER_THAN_OR_EQUALS, maxInstantTime)) {
- filenameToSizeMap.put(pathInfo.getPath().getName(),
pathInfo.getLength());
+ if (pathInfo.getLength() > 0 || !skipZeroSizeFiles) {
+ filenameToSizeMap.put(pathInfo.getPath().getName(),
pathInfo.getLength());
+ } else {
+ log.warn("Skipping zero-size data file during MDT bootstrap:
{}", pathInfo.getPath());
Review Comment:
minor: this WARN can get noisy if a partition has many zero-size files; an
aggregated per-directory WARN with the count would be quieter. Take it or leave
it.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -1114,6 +1118,10 @@ private List<DirectoryInfo>
listAllPartitionsFromFilesystem(String initializatio
}
}
+ if (skipZeroSizeFiles) {
+ final long zeroSizeCount = totalZeroSizeFiles;
+ metrics.ifPresent(m ->
m.incrementMetric("skipped_zero_size_files_on_initialize", zeroSizeCount));
Review Comment:
nit: metric names live as constants in `HoodieMetadataMetrics`
(`REBOOTSTRAP_STR`, `BOOTSTRAP_ERR_STR`, ...). Can we add a
`SKIPPED_ZERO_SIZE_FILES_ON_INITIALIZE_STR` there and reference it? Also fine
to only emit when `zeroSizeCount > 0`.
--
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]
