nsivabalan commented on code in PR #7738:
URL: https://github.com/apache/hudi/pull/7738#discussion_r1084642273
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java:
##########
@@ -295,4 +295,9 @@ public static Option<HoodieInstant>
getEarliestInstantForMetadataArchival(
return Option.empty();
}
}
+
+ public static Option<String> getFirstNotCompleted(HoodieTimeline timeline) {
Review Comment:
we should make this method getFirstInstant(timeline).
essentially irrespective fo whether its complete or inflight, first instant
in write timeline.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/dto/FileGroupDTO.java:
##########
@@ -56,7 +57,8 @@ public static FileGroupDTO fromFileGroup(HoodieFileGroup
fileGroup) {
public static HoodieFileGroup toFileGroup(FileGroupDTO dto,
HoodieTableMetaClient metaClient) {
HoodieFileGroup fileGroup =
- new HoodieFileGroup(dto.partition, dto.id,
TimelineDTO.toTimeline(dto.timeline, metaClient));
Review Comment:
we might need to fix FileGroupDTO to hold the first instant as well.
##########
hudi-cli/src/test/java/org/apache/hudi/cli/integ/ITTestRepairsCommand.java:
##########
@@ -262,6 +266,7 @@ public void
testDeduplicateNoPartitionWithInserts(HoodieTableType tableType) thr
connectTableAndReloadMetaClient(tablePath);
HoodieTableFileSystemView fsView = new
HoodieTableFileSystemView(metaClient,
metaClient.getActiveTimeline().getCommitsTimeline().filterCompletedInstants(),
+
TimelineUtils.getFirstNotCompleted(metaClient.getActiveTimeline().getCommitsTimeline()),
Review Comment:
yeah, thought about it. couple of reasons.
1. at times we do send the fileGroup fully to timeline server on which case
we are sending entire timeline (not just completed) and might take up more
space.
2. also, we really don't need entire timeline. we just need the first Active
instant. and hence took this route. Also, most of FileSystemView code assume
its completed timeline and hence didn't want to change that assumption.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/view/AbstractTableFileSystemView.java:
##########
@@ -104,9 +106,9 @@ private String getPartitionPathFor(HoodieBaseFile baseFile)
{
/**
* Initialize the view.
*/
- protected void init(HoodieTableMetaClient metaClient, HoodieTimeline
visibleActiveTimeline) {
+ protected void init(HoodieTableMetaClient metaClient, HoodieTimeline
visibleActiveTimeline,Option<String> firstNotCompleted) {
Review Comment:
nit: firstActiveInstant
--
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]
