Skip to content

[flink] Fix batch fallback generating mixed split types for primary-key tables#3296

Open
matrixsparse wants to merge 1 commit into
apache:mainfrom
matrixsparse:feature/fix-batch-fallback-mixed-splits
Open

[flink] Fix batch fallback generating mixed split types for primary-key tables#3296
matrixsparse wants to merge 1 commit into
apache:mainfrom
matrixsparse:feature/fix-batch-fallback-mixed-splits

Conversation

@matrixsparse
Copy link
Copy Markdown
Contributor

@matrixsparse matrixsparse commented May 10, 2026

Summary

Follow-up fix for #3208.

For primary-key tables in batch mode, when no lake snapshot exists, the previous
fallback logic had two issues:

  1. Mixed split types: initPartitionedSplits() / initNonPartitionedSplits()
    may produce both HybridSnapshotLogSplit and LogSplit, which the Flink
    connector does not support merging.

  2. Missing stopping offset: getLogSplit() used the 3-parameter LogSplit
    constructor without stopping offset, causing the reader to never stop in
    batch mode.

Changes

  • Use initLogTablePartitionSplits() / getLogSplit() in the fallback path
    to generate uniform LogSplit for all buckets.
  • Add stoppingOffsetsInitializer.getBucketOffsets() in getLogSplit() to
    provide stopping offsets for batch mode bounded reads.

@matrixsparse matrixsparse force-pushed the feature/fix-batch-fallback-mixed-splits branch from 09b5acd to 7ac1f8c Compare May 10, 2026 07:45
@matrixsparse
Copy link
Copy Markdown
Contributor Author

Hi @luoyuxia, this is a follow-up fix for the mixed split types issue you mentioned in #3208.

Could you PTAL? Thanks! cc @fresh-borzoni

Copy link
Copy Markdown
Member

@fresh-borzoni fresh-borzoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matrixsparse Ty, LGTM in general, one comment:

It matches spark logic, but at the same time this scenario is a bit dangerous - we have a big table that was never tiered to lake, then we decide to tier it to lake and run batched query through this fallback, instead of using kv_snapshot and replaying log on top, we read from earliest which is potentially a lot of records. So it's not very efficient and potentially OOM prone.

Let's file an issue about this to address separately for Spark and Flink?
cc @luoyuxia WDYT about this plan?

Copy link
Copy Markdown
Contributor

@luoyuxia luoyuxia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matrixsparse Thanks for the quick fix. Left on comments. PTAL

// Use log-only splits to avoid generating mixed split
// types (HybridSnapshotLogSplit + LogSplit) for
// primary-key tables, which is not supported.
splits = this.initLogTablePartitionSplits(partitions);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note it'll just genereate log split without stopping offset which will then nerver stop..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matrixsparse Good catch! Updated getLogSplit() to use stoppingOffsetsInitializer.getBucketOffsets() and pass the stopping offset to the 4-parameter LogSplit constructor. PTAL.

@fresh-borzoni
Copy link
Copy Markdown
Member

@matrixsparse @luoyuxia filed followups:
#3327
#3317 - resolved

Copy link
Copy Markdown
Member

@fresh-borzoni fresh-borzoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants