You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "zhongyujiang (via GitHub)" <gi...@apache.org> on 2026/06/22 13:06:36 UTC
[PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
zhongyujiang opened a new pull request, #16929:
URL: https://github.com/apache/iceberg/pull/16929
## Summary
`DataIterator.seek()` overwrites `fileOffset` after `updateCurrentIterator()` has correctly advanced it past files skipped by residual filters. This causes checkpoint to record a wrong file offset, leading to duplicate consumption or `IllegalStateException` on recovery.
This is a port of #10567 (Flink 1.19) to Flink 2.1.
## The Bug
In `seek(startingFileOffset, startingRecordOffset)`, the call to `updateCurrentIterator()` may advance `fileOffset` beyond `startingFileOffset` when head files are skipped (e.g., their row-group statistics allow the residual filter to eliminate all rows). However, `seek()` then unconditionally executes:
```java
fileOffset = startingFileOffset;
recordOffset = startingRecordOffset;
```
This overwrites the correct value with a stale one. For example, if `seek(0, 0)` is called and file 0 is skipped by the residual filter, `updateCurrentIterator()` correctly sets `fileOffset = 1` (pointing to file 1). But the assignment resets it to 0.
**Consequence:** Checkpoint records `fileOffset = 0` instead of `1`. On recovery:
- If file 1 has fewer records than `recordOffset`, throws `IllegalStateException: Invalid starting record offset`
- Otherwise, reads from the wrong file → duplicate data
## The Fix
1. Increment `fileOffset` in the skip-files loop (so it tracks position as files are skipped)
2. Remove the incorrect `fileOffset = startingFileOffset` / `recordOffset = startingRecordOffset` assignments at the end of `seek()` — these values are already correct through incremental updates
`recordOffset` is also naturally correct after the skip-records loop because `next()` increments it on each call, and the loop is guarded by `currentFileHasNext()` so no file-switch (which would reset `recordOffset`) can occur mid-skip.
## Tests
- `testDataIteratorWithResidualFilter`: validates that `fileOffset` matches the actual file being read when a residual filter is applied
- `testInitializationWithHeadFilesSkipped`: directly asserts that `fileOffset = 1` after `seek(0, 0)` when file 0 is skipped by residual filter — this fails without the fix
cc @pvary @stevenzwu Could you please review? This is a resubmission of #10567 ported to the latest Flink version (2.1), with an improved description. The underlying bug and fix are identical.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16929:
URL: https://github.com/apache/iceberg/pull/16929#discussion_r3466611722
##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/source/DataIterator.java:
##########
@@ -103,9 +104,6 @@ public void seek(int startingFileOffset, long startingRecordOffset) {
combinedTask));
}
}
-
- fileOffset = startingFileOffset;
- recordOffset = startingRecordOffset;
Review Comment:
Thanks!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "zhongyujiang (via GitHub)" <gi...@apache.org>.
zhongyujiang commented on code in PR #16929:
URL: https://github.com/apache/iceberg/pull/16929#discussion_r3464078101
##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/source/DataIterator.java:
##########
@@ -103,9 +104,6 @@ public void seek(int startingFileOffset, long startingRecordOffset) {
combinedTask));
}
}
-
- fileOffset = startingFileOffset;
- recordOffset = startingRecordOffset;
Review Comment:
The `recordOffset` assignment is unnecessary here because `next()` already increments `recordOffset` correctly during the skip-records loop above, so it naturally reaches the correct value by the end of the loop.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary merged PR #16929:
URL: https://github.com/apache/iceberg/pull/16929
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #16929:
URL: https://github.com/apache/iceberg/pull/16929#issuecomment-4788628056
Merged to main.
Thanks @zhongyujiang for the PR and for pushing to get this over the finish line. 🚀
Please don't forget to create a backport PR for the other Flink versions
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "zhongyujiang (via GitHub)" <gi...@apache.org>.
zhongyujiang commented on PR #16929:
URL: https://github.com/apache/iceberg/pull/16929#issuecomment-4776384600
@wombatu-kun Sure, I'll port the fix to other Flink versions after this PR gets approved.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #16929:
URL: https://github.com/apache/iceberg/pull/16929#issuecomment-4777347467
I still would like to see some end-to-end tests
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16929:
URL: https://github.com/apache/iceberg/pull/16929#discussion_r3460722232
##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/source/DataIterator.java:
##########
@@ -103,9 +104,6 @@ public void seek(int startingFileOffset, long startingRecordOffset) {
combinedTask));
}
}
-
- fileOffset = startingFileOffset;
- recordOffset = startingRecordOffset;
Review Comment:
Why did we remove the `recordOffset` setting?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "zhongyujiang (via GitHub)" <gi...@apache.org>.
zhongyujiang commented on PR #16929:
URL: https://github.com/apache/iceberg/pull/16929#issuecomment-4777807796
@pvary Unfortunately, I was unable to create a full end-to-end test in unit tests, because triggering this bug requires the reader to fail mid-way through reading a task and then recover from a checkpoint — I don't know how to reproduce this scenario in a unit test.
However, you can see that `testInitializationWithHeadFilesSkipped` is an end-to-end test of DataIterator that does not involve checkpointing. After executing dataIterator.seek(0, 0), the reader is actually positioned at fileOffset=1, recordOffset=0, but the current logic resets it to fileOffset=0, recordOffset=0.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on PR #16929:
URL: https://github.com/apache/iceberg/pull/16929#issuecomment-4769532155
The fix looks correct. Since the production change is a single line per version and the test scaffolding ports cleanly, consider also fixing `flink/v1.20` and `flink/v2.0` in this same PR - both still carry the identical bug, so folding them in here avoids two separate backport PRs.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "zhongyujiang (via GitHub)" <gi...@apache.org>.
zhongyujiang commented on PR #16929:
URL: https://github.com/apache/iceberg/pull/16929#issuecomment-4789531043
@pvary @wombatu-kun Thanks for reviewing!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Flink 2.1: Fix file offset mismatch in DataIterator.seek() when files are skipped [iceberg]
Posted by "zhongyujiang (via GitHub)" <gi...@apache.org>.
zhongyujiang commented on PR #16929:
URL: https://github.com/apache/iceberg/pull/16929#issuecomment-4778760037
Added `testSeekResumesCorrectlyAfterHeadFilesSkipped` which simulates a checkpoint/restore scenario to reproduce the bug. The test constructs [F0(filtered), F1(1 record), F2(2 records)], reads F1 + F2 record0, captures the checkpoint position, then creates a new DataIterator and seeks to that position. Without the fix, the incorrect fileOffset causes the restored iterator to re-read F2 record0 (duplicate data), which fails the assertion.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org