suryaprasanna commented on code in PR #9007:
URL: https://github.com/apache/hudi/pull/9007#discussion_r1264638709
##########
hudi-common/src/main/java/org/apache/hudi/common/table/view/AbstractTableFileSystemView.java:
##########
@@ -787,6 +803,21 @@ public final Stream<FileSlice> getLatestFileSlices(String
partitionStr) {
}
}
+ @Override
+ public final Stream<FileSlice> getLatestFileSlices(String partitionStr,
boolean includePending) {
Review Comment:
As @danny0405 pointed out in one of the comments, some of these are test
APIs. If I make one of them dependent on the other, people may start using
them. Not sure what path to follow, so for now made these APIs as separate, let
me know what your thoughts are on this.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/view/FileSystemViewManager.java:
##########
@@ -137,7 +137,7 @@ public void close() {
*/
private static RocksDbBasedFileSystemView
createRocksDBBasedFileSystemView(SerializableConfiguration conf,
FileSystemViewStorageConfig viewConf, HoodieTableMetaClient metaClient) {
- HoodieTimeline timeline =
metaClient.getActiveTimeline().filterCompletedAndCompactionInstants();
+ HoodieTimeline timeline = metaClient.getActiveTimeline();
Review Comment:
With the current change I went though the caller's caller and fixed at all
the places.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieFileGroup.java:
##########
@@ -53,6 +53,7 @@ public static Comparator<String>
getReverseCommitTimeComparator() {
/**
* Timeline, based on which all getter work.
+ * This should be a write timeline that contains either completed instants
or pending compaction instants.
Review Comment:
As agreed offline will be using fileSystemViewTimeline variable, since we
are using getFileSystemViewTimeline API.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/view/IncrementalTimelineSyncFileSystemView.java:
##########
@@ -448,14 +523,19 @@ protected void applyDeltaFileSlicesToPartitionView(String
partition, List<Hoodie
throw new IllegalStateException("Unknown diff apply mode=" + mode);
}
- HoodieTimeline timeline = deltaFileGroups.stream().map(df ->
df.getTimeline()).findAny().get();
+ // Here timeline is fetched from the existing file groups so it is
automatically completedWriteAndCompactionTimeline
+ HoodieTimeline completedWriteAndCompactionTimeline =
deltaFileGroups.stream().map(df -> df.getTimeline()).findAny().get();
List<HoodieFileGroup> fgs =
- buildFileGroups(viewDataFiles.values().stream(),
viewLogFiles.values().stream(), timeline, true);
+ buildFileGroups(viewDataFiles.values().stream(),
viewLogFiles.values().stream(), completedWriteAndCompactionTimeline, true);
storePartitionView(partition, fgs);
}
@Override
public HoodieTimeline getTimeline() {
return visibleActiveTimeline;
}
+
+ public boolean isLastIncrementalSyncSuccessful() {
Review Comment:
Good idea, marked it.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieFileGroup.java:
##########
@@ -178,7 +203,15 @@ public Option<HoodieBaseFile> getLatestDataFile() {
* Obtain the latest file slice, upto a instantTime i.e <= maxInstantTime.
*/
public Option<FileSlice> getLatestFileSliceBeforeOrOn(String maxInstantTime)
{
- return Option.fromJavaOptional(getAllFileSlices().filter(slice ->
compareTimestamps(slice.getBaseInstantTime(), LESSER_THAN_OR_EQUALS,
maxInstantTime)).findFirst());
+ return getLatestFileSliceBeforeOrOn(maxInstantTime, false);
+ }
+
+ /**
+ * Obtain the latest file slice, upto a instantTime i.e <= maxInstantTime.
+ */
+ public Option<FileSlice> getLatestFileSliceBeforeOrOn(String maxInstantTime,
boolean includePending) {
Review Comment:
I agree. Core writer flow does not use them, some are used by query engines.
They are able to get it working in a weird way, like some times they pass
active timeline to the file system view and sometimes they pass write timeline
etc.
--
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]
