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:
[email protected]
