Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Exhausted filtered-empty lake iterators are not closed, and checkpoint-restore behavior lacks regression coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR adds Flink streaming consumption for historical partition changelogs on supported primary-key tables.
Changes:
- Discovers and subscribes to
__historical__partitions. - Groups historical lake snapshots with bucket changelog offsets.
- Adds scanner, generator, integration, and documentation coverage.
File summaries
| File | Description |
|---|---|
FlinkSourceEnumerator.java |
Historical partition discovery and split initialization |
LakeSplitGenerator.java |
Historical lake/log hybrid split planning |
SeekableLakeSnapshotSplitScanner.java |
Skips empty lake splits |
HistoricalPartitionITCase.java |
End-to-end historical changelog coverage |
LakeSplitGeneratorTest.java |
Hybrid split unit tests |
SeekableLakeSnapshotSplitScannerTest.java |
Empty-split scanner test |
website/docs/engine-flink/reads.md |
Historical changelog documentation |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
9c13717 to
454f591
Compare
fd46762 to
38004e8
Compare
Consume historical partition logs for streaming log and primary-key tables. Pair primary-key lake baselines with historical bucket logs while retaining independent lake splits for log tables. Cover full and earliest startup, partition filtering, historical writes, and reader checkpoint recovery across business-partition lake splits. Document streaming-read support and configuration reload requirements. Co-Authored-By: Codex <noreply@openai.com> AI-Model: gpt-6 AI-Contributed/Feature: 90/90 AI-Contributed/UT: 394/394
38004e8 to
3d6ade6
Compare
|
@beryllw @loserwang1024 Could you please help review this pr |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
6 open issues
For non-historical partitions, iflakeSplitsexist butsnapshotLogOffsetis missing/negative,… · New These retries assert an exact emitted-record count, which is brittle: ifpollNextemits more than… · New Same resource-management issue here:UpsertWriteris not closed. Wrap the writer in… · New These retries assert an exact emitted-record count, which is brittle: ifpollNextemits more than… · NewUpsertWriteris created but never closed. If it holds buffers/network resources, this can leak… · New This doc update states that Flink streaming reads consume historical changelogs for both log tables… · New
2 resolved since last review
Review effort: Lite
loserwang1024
left a comment
There was a problem hiding this comment.
I left some comment
| - **Reads:** Primary-key point lookups continue to resolve rows after the original Fluss partition | ||
| no longer exists. Historical reads currently support only primary-key point lookups. | ||
| no longer exists. [Flink streaming reads](../../engine-flink/reads.md#streaming-read) | ||
| also consume historical changelogs for both log tables and primary-key tables. |
There was a problem hiding this comment.
Could we document what happens when a partition expires while the job is running? A historical update can arrive before an older record for the same key, and any unread records in the expired partition may be skipped.
There was a problem hiding this comment.
Added a warning in fc40c71 covering out-of-order updates and skipped unread records when a partition expires, including the impact on downstream materialized results and aggregations. It also clarifies that these limitations apply to both full and changelog-only startup modes.
| After enabling or disabling `table.datalake.historical-partition.enabled`, restart any existing | ||
| writer and lookup jobs that require historical partition access so that their clients reload the | ||
| updated table configuration. | ||
| writer, lookup, and Flink streaming read jobs that require historical partition access so that they |
There was a problem hiding this comment.
Could we add a note about restoring from an older checkpoint? If historical is discovered as a new partition after restore, the reader starts at the earliest retained offset, even with latest configured. This can replay changes already reflected in downstream state.
There was a problem hiding this comment.
Added a note in fc40c71: if an older checkpoint has no historical partition offsets and the source discovers __historical__ as new after restore, it starts at the earliest retained offset, even with scan.startup.mode = latest. The note explicitly calls out possible replay of changes already reflected in downstream state.
Document out-of-order updates and skipped unread records when a partition expires during streaming reads. Explain replay from the earliest retained offset when restoring a checkpoint without historical partition offsets. Co-Authored-By: Codex <noreply@openai.com> AI-Model: gpt-6 AI-Contributed/Feature: 17/17 AI-Contributed/UT: 0/0
|
@loserwang1024 Thanks for your review. Comments have been addressed. PTAL |


Purpose
Linked issue: Related to #4371. This PR covers consumption of existing historical partitions; runtime partition retirement remains outside its scope.
Flink currently discovers ordinary partitions only, so changes written through the internal historical partition are invisible to streaming consumers. Subscribe to the existing historical changelog for primary-key tables with historical partition access enabled. Keep original business partition values and apply SQL partition predicates to the records.
Scope and correctness limits
This change assumes that the ordinary partitions being read do not transition to historical partitions while the job runs, including during full startup. This is a usage precondition, not an enforced guard. Long-running jobs on auto-partitioned tables can encounter this boundary.
Performance and retention limits
min(historical bucket count, source parallelism); partitions within a bucket are read sequentially. This can reduce snapshot parallelism compared with independently assigned lake splits, and slow snapshots delay log output on the same reader. Changelog-only startup has no additional snapshot grouping cost; it remains subject to the existing per-bucket log parallelism limit.Brief change log
MetadataUpdaterafter pruning ordinary partitions. PreserveAdmin.listPartitionInfos()semantics and bypass KV-snapshot initialization for the historical partition.LakeSnapshotAndFlussLogSplitand the existing generation and reader paths.API and Format
No public API, RPC, storage format, split serializer version, or checkpoint state format changes.
Admin.listPartitionInfos()continues to list ordinary partitions only.An old checkpoint from a source without historical subscription has no historical offset. If the historical partition is discovered as new after restoration, the existing discovery rule starts it at the earliest retained offset, which can replay history already represented in downstream state. This PR does not provide checkpoint-upgrade offset migration or deduplication; an unchanged state format is not an upgrade-consistency guarantee.
Documentation
Document historical changelog consumption in
website/docs/engine-flink/reads.md, including the retirement ordering limitation, existing unread-log loss, snapshot parallelism tradeoff, shared-log filtering cost, and retention boundary.