ashokkumar-allu commented on PR #19299: URL: https://github.com/apache/hudi/pull/19299#issuecomment-5080464241
@nsivabalan — the three walk-throughs made the composition design's value very concrete, especially how the three breakage points collapse to one `instanceof` branch at a natural type boundary. The RFC now adopts the `MultiSourceFormatAdapter` composition model in full: - **`InputBatch`, `Source`, and `SourceFormatAdapter` public APIs are byte-for-byte unchanged.** No `getBatches()`, no fail-loud `getBatch()`, no new `Source` subclass. The Test Plan's "regression / nothing broke" item verifies this with a build-time signature check on those APIs. - **`MultiSourceFormatAdapter`** is composition, not inheritance (Section 1, L135–154). Constructor takes `List<SourceFormatAdapter> perSourceAdapters` (one per configured index), a `MultiTableCheckpointManager`, and the source-identifier registry. `fetchNewDataInRowFormat(...)` returns `List<InputBatch<Dataset<Row>>>` in index order; `fetchNewDataInAvroFormat(...)` throws `UnsupportedOperationException` (v1 is row-only, matching your walk-through 3 shape). - **`StreamSync` branches once** on `instanceof MultiSourceFormatAdapter` at the top of `fetchNextBatchFromSource` (Section 9, L435–480). The sequence diagram (L484–531) shows the two branches producing the same `(transformed, checkpoint)` pair; the rest of the pipeline — transformer error-event processing, schema deduction, row-writer vs Avro path, write, commit — is identical for both. - **Per-source error events** are handled inside each underlying single-source `SourceFormatAdapter` — the existing well-tested code path, invoked N times. No `processErrorEvents` branch is added inside `SourceFormatAdapter`. - **The extensibility argument** from your comment carries into the **Scope** section (L120–125): homogeneous multi-source of any type (N Kafka, N JSON, N JDBC…) and heterogeneous multi-source (mixed Hudi + Kafka + JSON) both fit architecturally; the outstanding work per new source type is per-source-type checkpoint encoding, not new adapter plumbing. - **The prior InputBatch/Source approach** is documented as item 3 of **Alternatives Considered** (L595–610), preserving your three-breakage-point analysis as the reason for rejection (fail-loud contract on a documented public API, three unconditional `getBatch()` call sites the compiler can't police, third-party surface hazard). One design consequence worth flagging that isn't a design change but is a slight inversion from your walk-through 3: composition means `Source.fetchNext`'s persist / schema-override hooks run once per per-source adapter (their natural single-source behavior) — they never see a multi-batch and no cross-source hook exists. I've treated this as a feature of the composition model rather than a limitation, since it keeps every hook local to its single source. Flag if you can think of a downstream consumer that would want a composite hook here. Thanks again for the walk-through — it substantially improved the design. -- 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]
