ad1happy2go commented on code in PR #20394:
URL: https://github.com/apache/hudi/pull/20394#discussion_r4398409629
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/LegacyArchivedMetaEntryReader.java:
##########
@@ -184,6 +185,20 @@ public void close() {
@Override
public boolean hasNext() {
+ List<Pair<HoodieInstant, Option<byte[]>>> instantAndDetails =
readNextInstantGroup();
+ // Legacy archives can hold instants that never completed (e.g. a
requested-only commit); they are not part of the timeline.
+ while (!instantAndDetails.isEmpty() &&
instantAndDetails.stream().noneMatch(instantAndDetail ->
instantAndDetail.getKey().isCompleted())) {
Review Comment:
This came from a production table, not only the test. A 0.14 table being
upgraded to 1.1.1 failed on its legacy archive, which holds
`20250711184246029__commit__REQUESTED__20250711184248572`: a commit archived
with only its REQUESTED record (no INFLIGHT, no COMPLETED).
The 0.x archiver can produce this. `HoodieTimelineArchiver.archive` converts
each state file separately and, when `hoodie.fail.on.timeline.archiving=false`,
logs and skips any file whose conversion fails. `deleteArchivedInstants` then
deletes every file in `instantsToArchive`, including the ones that were never
written to the archive. So a commit whose INFLIGHT and COMPLETED files failed
to convert ends up in the archive with only its REQUESTED record, and gone from
the active timeline.
I agree the archiver never chooses a pending instant to archive. This is a
partially written instant, not a pending one.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/timeline/LegacyArchivedMetaEntryReader.java:
##########
@@ -184,6 +185,20 @@ public void close() {
@Override
public boolean hasNext() {
+ List<Pair<HoodieInstant, Option<byte[]>>> instantAndDetails =
readNextInstantGroup();
+ // Legacy archives can hold instants that never completed (e.g. a
requested-only commit); they are not part of the timeline.
+ while (!instantAndDetails.isEmpty() &&
instantAndDetails.stream().noneMatch(instantAndDetail ->
instantAndDetail.getKey().isCompleted())) {
Review Comment:
Done in ce727a553c3. The grouping loop in `hasNext()` now drops a group
without a completed record as soon as the next instant starts (and at the end
of the stream), so such a group is never returned or passed to
`fromInstantAndDetails`. The separate filtering pass is gone and `hasNext()` is
back to its original shape. The records still have to be read first, since the
completed record is the last one in its group. The test now also covers an
incomplete instant as the last record read.
--
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]
