vinothchandar commented on code in PR #13216:
URL: https://github.com/apache/hudi/pull/13216#discussion_r2162535546
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java:
##########
@@ -1166,14 +1166,18 @@ private void
clearMetadataTablePartitionsConfig(Option<MetadataPartitionType> pa
public HoodieTableMetadata getMetadataTable() {
if (metadata == null) {
- HoodieMetadataConfig metadataConfig = HoodieMetadataConfig.newBuilder()
- .fromProperties(config.getMetadataConfig().getProps())
- .build();
- metadata = HoodieTableMetadata.create(context, metaClient.getStorage(),
metadataConfig, config.getBasePath());
+ metadata = refreshAndGetTableMetadata();
Review Comment:
nts: should we lock and make this thread safe
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/CopyOnWriteRollbackActionExecutor.java:
##########
@@ -67,11 +68,23 @@ protected List<HoodieRollbackStat>
executeRollback(HoodieRollbackPlan hoodieRoll
if (instantToRollback.isCompleted()) {
LOG.info("Unpublishing instant " + instantToRollback);
+ table.getMetaClient().getTableFormat().rollback(instantToRollback,
table.getContext(), table.getMetaClient(), table.getViewManager());
+ // Revert the completed instant to inflight in native format.
resolvedInstant = activeTimeline.revertToInflight(instantToRollback);
// reload meta-client to reflect latest timeline status
table.getMetaClient().reloadActiveTimeline();
}
+ // If instant is inflight but marked as completed in native format, delete
the completed instant from storage.
Review Comment:
nts: to review closely
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/StreamSync.java:
##########
@@ -456,6 +457,7 @@ HoodieTableMetaClient
initializeEmptyTable(HoodieTableMetaClient.TableBuilder ta
Boolean.parseBoolean(HIVE_STYLE_PARTITIONING_ENABLE.defaultValue())))
.setUrlEncodePartitioning(props.getBoolean(URL_ENCODE_PARTITIONING.key(),
Boolean.parseBoolean(URL_ENCODE_PARTITIONING.defaultValue())))
+
.setTableFormat(props.getProperty(HoodieTableConfig.TABLE_FORMAT.key(),
NativeTableFormat.TABLE_FORMAT))
Review Comment:
ensure every writer path - has this property wired in. i.e wherever the
table version is set.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java:
##########
@@ -305,6 +324,8 @@ public interface HoodieActiveTimeline extends
HoodieTimeline {
*/
HoodieInstant transitionClusterInflightToComplete(boolean shouldLock,
HoodieInstant inflightInstant, HoodieReplaceCommitMetadata metadata);
+ HoodieInstant transitionClusterInflightToComplete(boolean shouldLock,
HoodieInstant inflightInstant, HoodieReplaceCommitMetadata metadata,
TableFormatCompletionAction tableFormatCompletionAction);
Review Comment:
we should try and avoid the overload?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/versioning/v1/ActiveTimelineV1.java:
##########
@@ -470,12 +491,28 @@ public HoodieInstant transitionReplaceInflightToComplete(
return commitInstant;
}
+ @Override
+ public HoodieInstant transitionReplaceInflightToComplete(boolean shouldLock,
HoodieInstant inflightInstant, HoodieReplaceCommitMetadata metadata,
+
TableFormatCompletionAction tableFormatCompletionAction) {
+ HoodieInstant completedInstant =
transitionReplaceInflightToComplete(shouldLock, inflightInstant, metadata);
+ tableFormatCompletionAction.execute(completedInstant);
+ return completedInstant;
+ }
+
@Override
public HoodieInstant transitionClusterInflightToComplete(boolean shouldLock,
HoodieInstant inflightInstant, HoodieReplaceCommitMetadata metadata) {
// In 0.x, no separate clustering action, reuse replace action.
return transitionReplaceInflightToComplete(shouldLock, inflightInstant,
metadata);
}
+ @Override
+ public HoodieInstant transitionClusterInflightToComplete(boolean shouldLock,
HoodieInstant inflightInstant, HoodieReplaceCommitMetadata metadata,
Review Comment:
UTs? and the other new methods?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/dto/TimelineDTO.java:
##########
@@ -44,11 +45,21 @@ public static TimelineDTO fromTimeline(HoodieTimeline
timeline) {
return dto;
}
+ public static TimelineDTO fromInstants(List<HoodieInstant> instants) {
Review Comment:
UTs?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/versioning/v2/ActiveTimelineV2.java:
##########
@@ -710,12 +746,12 @@ public <T> void createFileInMetaPath(String filename,
Option<T> metadata, boolea
}
}
- protected <T> void createCompleteFileInMetaPath(boolean shouldLock,
HoodieInstant instant, Option<T> metadata) {
+ protected <T> long createCompleteFileInMetaPath(boolean shouldLock,
HoodieInstant instant, Option<T> metadata) {
Option<HoodieInstantWriter> writerOption =
getHoodieInstantWriterOption(this, metadata);
TimeGenerator timeGenerator = TimeGenerators
.getTimeGenerator(metaClient.getTimeGeneratorConfig(),
metaClient.getStorageConf());
- timeGenerator.consumeTime(!shouldLock, currentTimeMillis -> {
- String completionTime = TimelineUtils.generateInstantTime(false,
timeGenerator);
+ return timeGenerator.consumeTime(!shouldLock, currentTimeMillis -> {
+ String completionTime = HoodieInstantTimeGenerator.formatDateUTC(new
Date(currentTimeMillis));
Review Comment:
same; time generation alert.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -720,6 +728,20 @@ public Option<TimelineLayoutVersion>
getTimelineLayoutVersion() {
: Option.empty();
}
+ public TableFormat getTableFormat(TimelineLayoutVersion layoutVersion) {
Review Comment:
UT?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/versioning/v2/ActiveTimelineV2.java:
##########
@@ -526,12 +560,14 @@ protected <T> void transitionStateToComplete(boolean
shouldLock, HoodieInstant f
throw new HoodieIOException(
"Could not rename " + fromInstantPath + " to " + toInstantPath);
}
+ return instantWithCompletionTime;
} else {
// Ensures old state exists in timeline
ValidationUtils.checkArgument(
metaClient.getStorage().exists(getInstantFileNamePath(fromInstantFileName)),
"File " + getInstantFileNamePath(fromInstantFileName) + " does not
exist!");
- createCompleteFileInMetaPath(shouldLock, toInstant, metadata);
+ String completionTime = HoodieInstantTimeGenerator.formatDateUTC(new
Date(createCompleteFileInMetaPath(shouldLock, toInstant, metadata)));
Review Comment:
nts: is this compatible with other changes we are doing to smoothen time
generation
--
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]
