mahsoodebrahim commented on code in PR #18696:
URL: https://github.com/apache/hudi/pull/18696#discussion_r3262920136
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -894,7 +862,88 @@ public void restoreToSavepoint(String savepointTime) {
SavepointHelpers.validateSavepointRestore(table, savepointTime);
}
- @Deprecated
+ /**
+ * Decides whether the metadata table (MDT) must be deleted before restoring
the data table to
+ * {@code targetInstant}. Returns true when restoring would leave the MDT in
an inconsistent
+ * state, specifically when any of the following holds:
+ * <ol>
+ * <li>The target is at or before the MDT's penultimate completed
compaction (when at least
+ * two compactions exist). The restore would otherwise succeed for the
data table but
+ * {@code finishRestore} would fail to sync rollbacks into an MDT with
no base file at or
+ * before the target time.</li>
+ * <li>The target is at or before the oldest completed compaction. We
cannot restore to before
+ * the oldest compaction because we don't have base files before that
time.</li>
+ * <li>The target is before the MDT timeline start (the relevant history
was archived away).</li>
+ * </ol>
+ * Returns false when the MDT directory does not exist (nothing to delete or
worry about).
+ * Wraps any IOException reading the MDT in a {@link HoodieException} so
genuine permission /
+ * network failures surface to the caller instead of being silently
swallowed.
+ */
+ protected boolean shouldDeleteMdtBeforeRestore(String targetInstant) {
+ String mdtBasePath = getMetadataTableBasePath(config.getBasePath());
+ try {
+ // Cheap existence check first to avoid constructing an MDT meta client
when there is no MDT.
+ if (!storage.exists(new StoragePath(mdtBasePath))) {
+ return false;
+ }
+ HoodieTableMetaClient mdtMetaClient = HoodieTableMetaClient.builder()
+ .setConf(storageConf.newInstance())
+ .setBasePath(mdtBasePath).build();
+ List<HoodieInstant> completedCompactions =
mdtMetaClient.getCommitTimeline()
+ .filterCompletedInstants().getInstants();
+ if (completedCompactions.size() >= 2) {
+ String penultimate =
completedCompactions.get(completedCompactions.size() - 2).requestedTime();
+ if (LESSER_THAN_OR_EQUALS.test(targetInstant, penultimate)) {
+ log.warn("Deleting MDT before restore to {}: target is at or before
penultimate MDT compaction {}",
+ targetInstant, penultimate);
+ return true;
+ }
+ }
+ Option<HoodieInstant> oldestMdtCompaction =
completedCompactions.isEmpty()
+ ? Option.empty() : Option.of(completedCompactions.get(0));
+ if (oldestMdtCompaction.isPresent()
+ && LESSER_THAN_OR_EQUALS.test(targetInstant,
oldestMdtCompaction.get().requestedTime())) {
+ log.warn("Deleting MDT before restore to {}: target is at or before
oldest MDT compaction {}",
+ targetInstant, oldestMdtCompaction.get().requestedTime());
+ return true;
+ }
+ if
(mdtMetaClient.getCommitsTimeline().isBeforeTimelineStarts(targetInstant)) {
+ log.warn("Deleting MDT before restore to {}: target is before MDT
timeline start", targetInstant);
+ return true;
+ }
+ return false;
+ } catch (IOException e) {
Review Comment:
Fixed alongside danny's comment — the method is renamed
`deleteMdtIfNecessaryBeforeRestore` and the return value is flipped so that
`true = MDT was deleted`. The name and the boolean now agree.
--
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]
