[DISCUSS] Add checkpoint id in FLIP-27 source SplitEnumerator::snapshotState()

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[DISCUSS] Add checkpoint id in FLIP-27 source SplitEnumerator::snapshotState()

Zhou, Brian
Hi all,


I'd like to start a discussion thread about adding a checkpointId parameter in the SplitEnumerator:: snapshotState() method [1] .

This is required for Pravega Flink connector[2] FLIP-27 source implementation. We want to use the ExternallyInducedSourceReader, and it requires the split enumerator to track the Flink checkpoint ID. I have created an issue[3] for this.



The change is simple, however, this changes the API of the interface which will influence other FLIP-27 source implementations, so I'd like to start a public discussion here about your ideas and concerns for this proposal.



[1] https://github.com/apache/flink/blob/a66487d4d2f77bb09b9961655cc5922c3e5c098c/flink-core/src/main/java/org/apache/flink/api/connector/source/SplitEnumerator.java#L76

[2] https://github.com/pravega/flink-connectors

[3] https://issues.apache.org/jira/browse/FLINK-22133


Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Add checkpoint id in FLIP-27 source SplitEnumerator::snapshotState()

Becket Qin
Hi Brian,

Passing the checkpoint ID to SplitEnumerator.snapshotState() seems
reasonable to me. So +1 to the change.

BTW, typically for such API changes, we expect a FLIP wiki [1] to be
included in the discussion thread.

Thanks,

Jiangjie (Becket) Qin

[1]
https://cwiki.apache.org/confluence/display/FLINK/Flink+Improvement+Proposals

On Wed, Apr 14, 2021 at 3:42 PM Zhou, Brian <[hidden email]> wrote:

> Hi all,
>
>
>
> I'd like to start a discussion thread about adding a checkpointId
> parameter in the SplitEnumerator:: snapshotState() method [1] .
>
> This is required for Pravega Flink connector[2] FLIP-27 source
> implementation. We want to use the ExternallyInducedSourceReader, and it
> requires the split enumerator to track the Flink checkpoint ID. I have
> created an issue[3] for this.
>
>
>
> The change is simple, however, this changes the API of the interface which
> will influence other FLIP-27 source implementations, so I’d like to start a
> public discussion here about your ideas and concerns for this proposal.
>
>
>
> [1]
> https://github.com/apache/flink/blob/a66487d4d2f77bb09b9961655cc5922c3e5c098c/flink-core/src/main/java/org/apache/flink/api/connector/source/SplitEnumerator.java#L76
>
> [2] https://github.com/pravega/flink-connectors
>
> [3] https://issues.apache.org/jira/browse/FLINK-22133
>
>
>
Reply | Threaded
Open this post in threaded view
|

RE: [DISCUSS] Add checkpoint id in FLIP-27 source SplitEnumerator::snapshotState()

Zhou, Brian
Hi Becket,

OK, Can you give me the write permission to the wiki? Then I can start to write a FLIP.

Thanks,
Brian

From: Becket Qin <[hidden email]>
Sent: Wednesday, April 14, 2021 5:46 PM
To: Zhou, Brian
Cc: [hidden email]
Subject: Re: [DISCUSS] Add checkpoint id in FLIP-27 source SplitEnumerator::snapshotState()


[EXTERNAL EMAIL]
Hi Brian,

Passing the checkpoint ID to SplitEnumerator.snapshotState() seems reasonable to me. So +1 to the change.

BTW, typically for such API changes, we expect a FLIP wiki [1] to be included in the discussion thread.

Thanks,

Jiangjie (Becket) Qin

[1] https://cwiki.apache.org/confluence/display/FLINK/Flink+Improvement+Proposals [cwiki.apache.org]<https://urldefense.com/v3/__https:/cwiki.apache.org/confluence/display/FLINK/Flink*Improvement*Proposals__;Kys!!LpKI!xwfAp_wvHkSKkMgl5G7Bqs5LfzpTphncMoWs8EXYQ7BqnBY_lc1TRjaa1Pgv$>

On Wed, Apr 14, 2021 at 3:42 PM Zhou, Brian <[hidden email]<mailto:[hidden email]>> wrote:
Hi all,


I'd like to start a discussion thread about adding a checkpointId parameter in the SplitEnumerator:: snapshotState() method [1] .

This is required for Pravega Flink connector[2] FLIP-27 source implementation. We want to use the ExternallyInducedSourceReader, and it requires the split enumerator to track the Flink checkpoint ID. I have created an issue[3] for this.



The change is simple, however, this changes the API of the interface which will influence other FLIP-27 source implementations, so I’d like to start a public discussion here about your ideas and concerns for this proposal.



[1] https://github.com/apache/flink/blob/a66487d4d2f77bb09b9961655cc5922c3e5c098c/flink-core/src/main/java/org/apache/flink/api/connector/source/SplitEnumerator.java#L76 [github.com]<https://urldefense.com/v3/__https:/github.com/apache/flink/blob/a66487d4d2f77bb09b9961655cc5922c3e5c098c/flink-core/src/main/java/org/apache/flink/api/connector/source/SplitEnumerator.java*L76__;Iw!!LpKI!xwfAp_wvHkSKkMgl5G7Bqs5LfzpTphncMoWs8EXYQ7BqnBY_lc1TRg_K-s7U$>

[2] https://github.com/pravega/flink-connectors [github.com]<https://urldefense.com/v3/__https:/github.com/pravega/flink-connectors__;!!LpKI!xwfAp_wvHkSKkMgl5G7Bqs5LfzpTphncMoWs8EXYQ7BqnBY_lc1TRuCix4x-$>

[3] https://issues.apache.org/jira/browse/FLINK-22133 [issues.apache.org]<https://urldefense.com/v3/__https:/issues.apache.org/jira/browse/FLINK-22133__;!!LpKI!xwfAp_wvHkSKkMgl5G7Bqs5LfzpTphncMoWs8EXYQ7BqnBY_lc1TRrokZrZu$>