satishkotha commented on a change in pull request #2784: URL: https://github.com/apache/hudi/pull/2784#discussion_r609094597
########## File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/MetadataConversionUtils.java ########## @@ -105,14 +114,15 @@ public static HoodieArchivedMetaEntry createMetaWrapper(HoodieInstant hoodieInst return archivedMetaWrapper; } - public static HoodieArchivedMetaEntry createMetaWrapper(HoodieInstant hoodieInstant, - HoodieCommitMetadata hoodieCommitMetadata) { - HoodieArchivedMetaEntry archivedMetaWrapper = new HoodieArchivedMetaEntry(); - archivedMetaWrapper.setCommitTime(hoodieInstant.getTimestamp()); - archivedMetaWrapper.setActionState(hoodieInstant.getState().name()); - archivedMetaWrapper.setHoodieCommitMetadata(convertCommitMetadata(hoodieCommitMetadata)); - archivedMetaWrapper.setActionType(ActionType.commit.name()); - return archivedMetaWrapper; + public static Option<HoodieRequestedReplaceMetadata> getRequestedReplaceMetadata(HoodieTableMetaClient metaClient, HoodieInstant pendingReplaceInstant) throws IOException { + final HoodieInstant requestedInstant = HoodieTimeline.getReplaceCommitRequestedInstant(pendingReplaceInstant.getTimestamp()); + + Option<byte[]> content = metaClient.getActiveTimeline().getInstantDetails(requestedInstant); + if (!content.isPresent() || content.get().length == 0) { + LOG.warn("No content found in requested file for instant " + pendingReplaceInstant); + return Option.of(new HoodieRequestedReplaceMetadata()); Review comment: Why not return Option.empty() and skip archival for this case? ########## File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/MetadataConversionUtils.java ########## @@ -72,9 +76,14 @@ public static HoodieArchivedMetaEntry createMetaWrapper(HoodieInstant hoodieInst HoodieReplaceCommitMetadata replaceCommitMetadata = HoodieReplaceCommitMetadata .fromBytes(metaClient.getActiveTimeline().getInstantDetails(hoodieInstant).get(), HoodieReplaceCommitMetadata.class); archivedMetaWrapper.setHoodieReplaceCommitMetadata(ReplaceArchivalHelper.convertReplaceCommitMetadata(replaceCommitMetadata)); + } else if (hoodieInstant.isInflight()) { + // inflight replacecommit files have the same meta data body as HoodieCommitMetadata Review comment: We also use replacecommit for 'clustering' operation. Clustering has empty replacecommit.inflight file, so this may not work ########## File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/AbstractTableFileSystemView.java ########## @@ -245,7 +245,7 @@ public final void reset() { bootstrapIndex = null; // Initialize with new Hoodie timeline. - init(metaClient, getTimeline()); + init(metaClient, metaClient.reloadActiveTimeline()); Review comment: IIUC, this is breaking some fundamental assumptions. There are many places where we pass "trimmed" timeline for time-travel queries etc. You are replacing that with all instants from active timeline, which is not desired. Is this change needed if we handle empty partitionToReplaceFileIds in archival? -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org