hudi-agent commented on code in PR #19497:
URL: https://github.com/apache/hudi/pull/19497#discussion_r3870961484
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieMetadataTableValidator.java:
##########
@@ -810,21 +811,69 @@ 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" + validateFilesInCommonPartitions(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
summarises the outcome.
+ *
+ * <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
Review Comment:
🤖 nit: could you rename this to `summarizeFileValidationForCommonPartitions`
(or similar)? Every other `validate*` method in this class signals failure by
throwing, so a future reader might expect this one to throw too — returning a
`String` summary is unexpected from the name alone.
<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]