danny0405 commented on issue #19774:
URL: https://github.com/apache/hudi/issues/19774#issuecomment-5449553257
I reproduced this and agree the fix should happen while the file group is
constructed, rather than by weakening `isFileSliceCommitted` or treating a
slice as committed merely because it contains a committed log.
The key issue is that `addLogFile` cannot choose the correct initial base
instant when it sees only the failed leading log. The filesystem view already
has the complete set of logs for the file group, so the proposed fix adds them
as a batch:
1. Sort all logs by delta-commit time inside `HoodieFileGroup`.
2. If there is no existing base/compaction slice, find the earliest
completed log and establish its instant as the initial slice base.
3. Add every log in the original delta-commit order. Earlier pending logs
then follow the existing pending-log rule and attach to the latest slice.
4. Keep `isFileSliceCommitted` unchanged; it still validates the slice
anchor, while the V8+ read-view filtering removes individual uncommitted files
from that valid slice.
For the reported sequence, the result becomes:
```text
raw slice t3: [t2 (failed), t3, t4]
visible slice t3: [t3, t4]
```
There is no separate slice anchored at the uncommitted `t2`.
<details>
<summary>Proposed production patch</summary>
```diff
diff --git
a/hudi-common/src/main/java/org/apache/hudi/common/model/HoodieFileGroup.java
b/hudi-common/src/main/java/org/apache/hudi/common/model/HoodieFileGroup.java
index 89088cb14190..38329fcbab69 100644
---
a/hudi-common/src/main/java/org/apache/hudi/common/model/HoodieFileGroup.java
+++
b/hudi-common/src/main/java/org/apache/hudi/common/model/HoodieFileGroup.java
@@ -32,6 +32,7 @@ import java.io.Serializable;
import java.util.Comparator;
import java.util.List;
import java.util.TreeMap;
+import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -117,7 +118,7 @@ public class HoodieFileGroup implements Serializable {
*
* <p>CAUTION: the log file must be added in sequence of the delta commit
time.
*/
- public void addLogFile(CompletionTimeQueryView completionTimeQueryView,
HoodieLogFile logFile) {
+ private void addLogFile(CompletionTimeQueryView completionTimeQueryView,
HoodieLogFile logFile) {
String baseInstantTime = getBaseInstantTime(completionTimeQueryView,
logFile);
if (!fileSlices.containsKey(baseInstantTime)) {
fileSlices.put(baseInstantTime, new FileSlice(fileGroupId,
baseInstantTime));
@@ -125,6 +126,28 @@ public class HoodieFileGroup implements Serializable {
fileSlices.get(baseInstantTime).addLogFile(logFile);
}
+ /**
+ * Add log files into the group.
+ *
+ * <p>When the group has no existing slice, the first completed log
establishes the initial slice before any logs
+ * are added. This allows an earlier pending log to follow the normal
pending-log rule and attach to the latest slice,
+ * instead of creating an uncommitted slice that would also hide later
committed logs.
+ */
+ public void addLogFiles(CompletionTimeQueryView completionTimeQueryView,
List<HoodieLogFile> logFiles) {
+ if (fileSlices.isEmpty()) {
+ List<HoodieLogFile> sortedLogFiles = logFiles.stream()
+
.sorted(HoodieLogFile.getLogFileComparator()).collect(Collectors.toList());
+ sortedLogFiles.stream()
+ .filter(logFile ->
completionTimeQueryView.isCompleted(logFile.getDeltaCommitTime()))
+ .findFirst()
+ .ifPresent(logFile ->
addNewFileSliceAtInstant(logFile.getDeltaCommitTime()));
+ sortedLogFiles.forEach(logFile -> addLogFile(completionTimeQueryView,
logFile));
+ } else {
+ logFiles.stream().sorted(HoodieLogFile.getLogFileComparator())
+ .forEach(logFile -> addLogFile(completionTimeQueryView, logFile));
+ }
+ }
+
@VisibleForTesting
public String getBaseInstantTime(CompletionTimeQueryView
completionTimeQueryView, HoodieLogFile logFile) {
diff --git
a/hudi-common/src/main/java/org/apache/hudi/common/table/view/AbstractTableFileSystemView.java
b/hudi-common/src/main/java/org/apache/hudi/common/table/view/AbstractTableFileSystemView.java
index 75118b90bb2c..f768c6880def 100644
---
a/hudi-common/src/main/java/org/apache/hudi/common/table/view/AbstractTableFileSystemView.java
+++
b/hudi-common/src/main/java/org/apache/hudi/common/table/view/AbstractTableFileSystemView.java
@@ -256,8 +256,7 @@ public abstract class AbstractTableFileSystemView
implements SyncableFileSystemV
}
if (logFiles.containsKey(fileId)) {
// this should work for both table versions >= 8 and lower.
-
logFiles.get(fileId).stream().sorted(HoodieLogFile.getLogFileComparator())
- .forEach(logFile -> group.addLogFile(completionTimeQueryView,
logFile));
+ group.addLogFiles(completionTimeQueryView, logFiles.get(fileId));
}
fileGroups.add(group);
});
```
</details>
Regression coverage constructs an MOR file group with `t2` inflight and
`t3`/`t4` completed, verifies that the raw view has exactly one slice based at
`t3` containing all three logs, and verifies that the V8+ visible view filters
only `t2` while retaining `t3`/`t4`. The unit regression deliberately supplies
the logs out of order to verify that sorting is owned by `addLogFiles`.
Compatibility notes:
* Existing base files and pending-compaction slices remain authoritative
because the initial-base lookup runs only when `fileSlices` is empty.
* If every log is pending, the existing provisional pending-slice behavior
is retained.
* Table-version-6/pre-V8 log naming continues to use the encoded base
instant; the existing version-6 slicing tests pass through the new batch API.
Focused result: `TestHoodieFileGroup` passes all 8 tests, including both
pre-V8 and V8+ slicing cases.
--
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]
