Skip to content

[FLINK-39722][pipeline-connector][fluss] Support Fluss CDC Yaml Source. - #4494

Merged
leonardBang merged 8 commits into
apache:masterfrom
loserwang1024:fluss-source
Aug 27, 2026
Merged

[FLINK-39722][pipeline-connector][fluss] Support Fluss CDC Yaml Source.#4494
leonardBang merged 8 commits into
apache:masterfrom
loserwang1024:fluss-source

Conversation

@loserwang1024

Copy link
Copy Markdown
Contributor

What is the purpose of this pull request?

Briefly describe the problem this PR fixes or the feature it introduces. Reference Flink JIRA ticket when possible.

Brief change log

Support Fluss yaml source.


Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

This change added tests and can be verified as follows:

  • Added/Updated unit tests in ...*
  • Added/Updated integration tests in ...*
  • Manually tested by ...

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

@fxbing

fxbing commented Aug 6, 2026

Copy link
Copy Markdown

Ran this PR (5a7ce32) end-to-end as a Fluss→Fluss pipeline, built against apache/fluss main (22cb1969). Build is fine and the PR's own tests are green (74 UT + 43 IT). Two issues showed up in E2E:

  1. Source deserializer doesn't handle ARRAY / MAP / ROW

FlussRecordDeserializer.convertFlussField covers the 16 scalar types and throws for everything else:

java.lang.UnsupportedOperationException: Unsupported Fluss data type for deserialization: ArrayType
at ...FlussRecordDeserializer.convertFlussField(FlussRecordDeserializer.java:396)
at ...FlussRecordDeserializer.convertFlussRowToCdcRecord(FlussRecordDeserializer.java:201)

Repro: a source table with tags ARRAY (same for MAP / ROW) — the job fails on the first record and goes into a restart loop. The sink side already supports these types via CdcAsFlussArray /
CdcAsFlussMap / CdcAsFlussRow, so the gap looks source-only.

  1. CreateTableEvent isn't reconciled against an existing sink table

If the sink table already exists with fewer columns than the source, FlussMetaDataApplier treats CreateTableEvent as create-if-not-exists and leaves the physical schema as is. Upstream-only columns are then
silently dropped while the job stays RUNNING with no error.

Repro: sink (id, payload), source (id, payload, extra_col) → the row lands as (1, 'cold-start') and extra_col is lost. The coordinator does pass the full schema down:

Step 3.5 - Corresponding schema changes are: [CreateTableEvent{... extra_col STRING ...}]

A worse variant: when the sink is missing a middle column (source (id, name, sale_quantity) vs sink (id, sale_quantity)), the write still succeeds with name projected away rather than failing fast.

Would it make sense for the metadata applier to reload the real TableInfo on CreateTableEvent, append missing nullable trailing columns, and fail fast when the difference can't be reconciled that way?

@loserwang1024

loserwang1024 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@fxbing

  1. Source deserializer doesn't handle ARRAY / MAP / ROW

support it now.

  1. CreateTableEvent isn't reconciled against an existing sink table

This is a sink behavior, thus is not included in this PR. I have raise a new issue to solve it: https://issues.apache.org/jira/browse/FLINK-40349

@leonardBang
leonardBang self-requested a review August 12, 2026 09:16
public SourceReader<T, FlussSplitBase> createReader(SourceReaderContext readerContext) {
FlussSourceReaderMetrics sourceReaderMetrics =
new FlussSourceReaderMetrics(readerContext.metricGroup());
WrapperFlussMetricRegistry metricRegistry =

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WrapperFlussMetricRegistry wraps Fluss METERs as WrapperFlussMeter, but it never registers a MetricViewUpdater (or otherwise calls update() on the underlying MeterView). WrapperFlussMeter#getRate() therefore only exposes the cached value.

The new source creates this registry in FlussSource#createReader, so scanner rate metrics such as fetchRequestsPerSecond and remoteFetch* will remain at their initial 0 even while records are being fetched. Could we register and close a MetricViewUpdater for this registry, as the Fluss metric integration requires?

@leonardBang leonardBang left a comment

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.

Thanks @loserwang1024 for the contribution, I left some comments

Hongshun Wang and others added 5 commits August 25, 2026 11:47
Co-Authored-By: Codex <noreply@openai.com>
AI-Model: gpt-5.6-sol
AI-Contributed/Feature: 1785/4380
AI-Contributed/UT: 934/3936
Co-Authored-By: Codex <noreply@openai.com>
AI-Model: gpt-5.6-sol
AI-Contributed/Feature: 225/225
AI-Contributed/UT: 88/429
tmp

Co-Authored-By: Codex <noreply@openai.com>
AI-Model: gpt-5.6-sol
AI-Contributed/Feature: 5/5
AI-Contributed/UT: 0/902
Co-Authored-By: Qoder <noreply@qoder.com>
Co-Authored-By: Codex <noreply@openai.com>
AI-Model: gpt-5.6-sol
AI-Contributed/Feature: 51/62
AI-Contributed/UT: 0/0
Co-Authored-By: Codex <noreply@openai.com>
AI-Model: gpt-5.6-sol
AI-Contributed/Feature: 208/208
AI-Contributed/UT: 296/296
Co-Authored-By: Codex <noreply@openai.com>
AI-Model: gpt-5.6-sol
AI-Contributed/Feature: 2/2
AI-Contributed/UT: 0/0
@github-actions github-actions Bot added the build label Aug 25, 2026

@leonardBang leonardBang left a comment

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.

Thanks for the update. I found three additional issues during the full review.

@@ -26,10 +26,6 @@
public interface TableDiscovererFactory extends ObjectIdDiscovererFactory<TableId> {

/** Compatibility alias for the old single identifier, now meaning storage-side type. */

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.

Could we keep the identifier() compatibility alias here? TableDiscovererFactory is @PublicEvolving, and the current master API intentionally retains this default method as the legacy name for type(). Removing it in an unrelated Fluss source PR can break third-party discoverer code compiled against the current SPI and also leaves the compatibility-alias Javadoc attached to objectIdClass(). Unless this removal is part of a separately reviewed SPI migration, restoring the alias would avoid an unnecessary compatibility regression.

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.

Since https://issues.apache.org/jira/browse/FLINK-39732 is for the same version of the Fluss YAML source, semantic simplicity takes priority.

List<FlussSplitBase> splits = new ArrayList<>();

// Get KV snapshots for this table/partition
KvSnapshots kvSnapshots =

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.

Could we acquire and checkpoint a KV snapshot lease before creating these hybrid splits? The split stores a concrete snapshot ID, but the Fluss server retains only a bounded number of completed snapshots, so an unleased snapshot may be pruned while a slow full scan is still running or before the job restores from a checkpoint/savepoint. I reproduced this with kv.snapshot.num-retained=1: after the enumerator checkpointed snapshot 0 and two newer snapshots were created, createBatchScanner(tableBucket, snapshotId) failed with KvSnapshotNotExistException; acquiring a lease made the same test pass. The native Fluss source persists its lease ID and releases snapshots only after their consumption is checkpointed. Could we implement the corresponding lease lifecycle here and add this recovery test?

@leonardBang leonardBang left a comment

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.

Thanks @loserwang1024 for the update and @fxbing for the review, LGTM now

@leonardBang

Copy link
Copy Markdown
Contributor

Will merge once CI green....

@leonardBang
leonardBang merged commit 5d86d11 into apache:master Aug 27, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants