hudi-agent commented on code in PR #19497:
URL: https://github.com/apache/hudi/pull/19497#discussion_r3901114401
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieMetadataTableValidator.java:
##########
@@ -810,21 +811,71 @@ List<String> validatePartitions(HoodieSparkEngineContext
engineContext, StorageP
}
}
if (misMatch.get()) {
+ // The partition lists genuinely disagree, so this run fails either
way. Before reporting it,
+ // validate the partitions both sides do agree on: throwing here skips
the per-partition file
+ // validation the caller would otherwise run, which leaves the
operator knowing only that the
+ // partition lists differ and nothing about whether the rest of the
table is consistent.
+ List<String> commonPartitions = new
ArrayList<>(allPartitionPathsFromFS);
+ commonPartitions.retainAll(allPartitionPathsMeta);
String message = "Compare Partitions Failed! " + " Additional "
+ additionalFromFS.size() + " partitions from FS, but missing from
MDT : \""
+ toStringWithThreshold(additionalFromFS, cfg.logDetailMaxLength)
+ "\" and additional " + actualAdditionalPartitionsInMDT.size()
+ " partitions from MDT, but missing from FS listing : \""
+ toStringWithThreshold(actualAdditionalPartitionsInMDT,
cfg.logDetailMaxLength)
+ "\".\n All " + allPartitionPathsFromFS.size() + " partitions
from FS listing "
- + toStringWithThreshold(allPartitionPathsFromFS,
cfg.logDetailMaxLength);
+ + toStringWithThreshold(allPartitionPathsFromFS,
cfg.logDetailMaxLength)
+ + "\n" + summarizeFileValidationForCommonPartitions(engineContext,
metaClient, commonPartitions, baseFilesForCleaning);
log.error(message);
throw new HoodieValidationException(message);
}
}
return allPartitionPathsMeta;
}
+ /**
+ * Validates the file listing for the partitions present on both sides and
returns a summary of the
+ * outcome. Unlike the other {@code validate} methods here, this one never
throws: it reports through
+ * its return value instead.
+ *
+ * <p>Only called once a partition mismatch has already been detected and
this run is going to fail.
+ * The result is reported alongside the mismatch rather than thrown, so that
the partition list
+ * difference stays the reported cause; a file level problem found here is
extra detail about a run
+ * that was failing anyway. Any error raised while validating a common
partition is captured for the
+ * same reason.
+ */
+ private String
summarizeFileValidationForCommonPartitions(HoodieSparkEngineContext
engineContext, HoodieTableMetaClient metaClient,
+ List<String>
commonPartitions, Set<String> baseFilesForCleaning) {
+ if (commonPartitions.isEmpty()) {
+ return "No partitions are common to FS listing and MDT, so there are no
file listings to compare.";
+ }
+ try (HoodieMetadataValidationContext metadataTableBasedContext =
+ new HoodieMetadataValidationContext(engineContext, props,
metaClient, true, cfg.viewStorageTypeForMetadata);
+ HoodieMetadataValidationContext fsBasedContext =
+ new HoodieMetadataValidationContext(engineContext, props,
metaClient, false, cfg.viewStorageTypeForFSListing)) {
+ List<String> failures = engineContext.parallelize(commonPartitions,
commonPartitions.size())
+ .map(partitionPath -> {
+ try {
+ validateFilesInPartition(metadataTableBasedContext,
fsBasedContext, partitionPath, baseFilesForCleaning);
+ return "";
Review Comment:
🤖 nit: using `""` as a sentinel for "no failure" is a little surprising —
could you return `null` here and change the filter to `Objects::nonNull`?
That's the more conventional Java idiom for "this item produced no result" in a
stream.
<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]
