lnbest0707 commented on PR #264: URL: https://github.com/apache/flink-connector-kafka/pull/264#issuecomment-4633706897
> > Hey @jubins thanks for you interest for the issue I created recently. When I called out the issue and created the Jira ticket, I got the proposal hence pushed this PR fix. > > IIUC, your PR looks like a start point as a framework, while this one is a concrete e2e implementations to cover real usage and some edge cases as well. As a result, would it make more sense to choose this PR as a follow up, thanks. > > hi @lnbest0707 Does your PR also include the framework-level changes from [apache/flink#28318](https://github.com/apache/flink/pull/28318), or does it only contain connector-specific changes? Looking at your implementation, I see it's focused on the Kafka connector side. However, the framework classes (SplitAssignmentTracker, SourceCoordinatorSerdeUtils) live in the main Flink repository Hi @jubins (cross post the discussion in the Jira Issue thread) No, [apache/flink-connector-kafka#264](https://github.com/apache/flink-connector-kafka/pull/264) intentionally contains connector-specific changes only. For this issue, DynamicKafkaSource already owns the relevant checkpoint state: reader split offsets and dynamic enumerator state. Retaining removed Kafka cluster state there is sufficient to restore progress when the cluster is re-added within the retention window. The SplitAssignmentTracker / SourceCoordinatorSerdeUtils changes in [apache/flink#28318](https://github.com/apache/flink/pull/28318) look like a broader framework proposal for generic Flink sources, but they are not a dependency for this Kafka connector fix. Also, from the current diff in [apache/flink-connector-kafka#265](https://github.com/apache/flink-connector-kafka/pull/265), I don’t think it is sufficient yet to fully handle the DynamicKafkaSource behavior needed here, especially the connector-specific removed-cluster reader/enumerator semantics. For the broader Flink framework change, I’m not sure whether there is already an issue/discussion and consensus for introducing that generic mechanism. -- 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]
