[FLINK-27399][Connector/Pulsar] Change initial consuming position setting logic for better handle the checkpoint. by syhily · Pull Request #19972 · apache/flink

syhily · 2022-06-15T16:32:46Z

What is the purpose of the change

Brief change log

This task modifies the flink-connector-pulsar module, adds some new mechanisms to set the initial consuming position.

Change StartCursor, add new useful methods, and deprecate the confused fromMessageTime() method.
Change StopCursor, and add new useful methods.
Introduce a new SplitAssigner for assigning the splits among the Pulsar readers. Make the partition assignment logic clear and testable.
Change the start position seeking mechanism from Pulsar consumer API to Pulsar admin API. Don't reset the start position when the topic has a subscription.

Verifying this change

This change is already covered by existing tests, such as:

PulsarSourceITCase
PulsarSourceEnumeratorTest
PulsarOrderedPartitionSplitReaderTest

We add new tests for new partition assign logic:

NormalSplitAssignerTest
SharedSplitAssignerTest

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (yes)
The serializers: (no)
The runtime per-record code paths (performance sensitive): (no)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
The S3 file system connector: (no)

Documentation

Does this pull request introduces a new feature? (yes)
If yes, how is the feature documented? (docs)

flinkbot · 2022-06-15T16:35:02Z

CI report:

7ea6b3c Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

imaffe

I reviewed until the NormlSplitAssigner. Will continue review tomorrow.

MartijnVisser · 2022-07-11T07:31:49Z

@PatrickRen Can you also have a look at this PR? I've understood that this PR should help resolve this blocker test stability FLINK-26721

MartijnVisser · 2022-07-12T13:31:39Z

@tisonkun I've understood from @wuchong that you might also want to help/have a look at this PR, therefore I've tagged you.

tisonkun · 2022-07-12T13:44:24Z

@MartijnVisser Thank you. I'll review the patch in this week. Actually I ever try to request myself as a reviewer but forget several times >_<

tisonkun

Generally looks good. The effective changes are:

Changing SplitAssignmentState to SplitAssigner
Create subscription via PulsarAdmin API instead of consumer API and seek.

These significant changes are reasonable from my perspective.

Comments inline.

MartijnVisser · 2022-07-21T12:30:31Z

@syhily Can you resolve the latest review comments?

…top cursor for better handle the consuming behaviors.

…ad seek every time. This should fix the wrong position setting.

…it stops at correct space

…r#16171

syhily · 2022-08-12T11:30:27Z

@tisonkun Finally, the ci turns green.

…ting logic for better handle the checkpoint. (apache#19972) * Change the initial start cursor and stop cursor to better handle the consuming behaviors. * Create the initial subscription instead seek every time. This should fix the wrong position setting. * Fix the wrong stop cursor, make sure it stops at the correct space * Drop Consumer.seek() for apache/pulsar#16171 (cherry picked from commit 18d21a0)

…ting logic for better handle the checkpoint. (apache#19972) * Change the initial start cursor and stop cursor to better handle the consuming behaviors. * Create the initial subscription instead seek every time. This should fix the wrong position setting. * Fix the wrong stop cursor, make sure it stops at the correct space * Drop Consumer.seek() for apache/pulsar#16171

…ting logic for better handle the checkpoint. (#19972) (#20565) * Change the initial start cursor and stop cursor to better handle the consuming behaviors. * Create the initial subscription instead seek every time. This should fix the wrong position setting. * Fix the wrong stop cursor, make sure it stops at the correct space * Drop Consumer.seek() for apache/pulsar#16171

…ting logic for better handle the checkpoint. (#19972) (#20564) * Change the initial start cursor and stop cursor to better handle the consuming behaviors. * Create the initial subscription instead seek every time. This should fix the wrong position setting. * Fix the wrong stop cursor, make sure it stops at the correct space * Drop Consumer.seek() for apache/pulsar#16171

…ting logic for better handle the checkpoint. (apache#19972) * Change the initial start cursor and stop cursor to better handle the consuming behaviors. * Create the initial subscription instead seek every time. This should fix the wrong position setting. * Fix the wrong stop cursor, make sure it stops at the correct space * Drop Consumer.seek() for apache/pulsar#16171

flinkbot added the component=Connectors/Pulsar label Jun 15, 2022

syhily changed the title ~~[FLINK-27399][Connector/Pulsar] Modify start cursor and stop cursor, change initial position setting logic.~~ [FLINK-27399][Connector/Pulsar] Change initial consuming position setting logic for better handle the checkpoint. Jun 15, 2022

syhily mentioned this pull request Jun 15, 2022

[Feature][FLINK-27399] SourceEvent protocol for limiting the Consumer.seek operation streamnative/flink#6

Closed

a49a reviewed Jun 21, 2022

View reviewed changes

Comment thread flink-python/pyflink/datastream/connectors/pulsar.py Outdated

syhily force-pushed the feature/new-seek-lifecycle branch 11 times, most recently from cbf393b to 1ae927b Compare July 8, 2022 18:34

imaffe reviewed Jul 10, 2022

View reviewed changes

Comment thread docs/content.zh/docs/connectors/datastream/pulsar.md

Comment thread ...rc/main/java/org/apache/flink/connector/pulsar/source/enumerator/PulsarSourceEnumerator.java

MartijnVisser requested a review from PatrickRen July 11, 2022 07:31

syhily force-pushed the feature/new-seek-lifecycle branch from 1ae927b to 12049bf Compare July 11, 2022 22:19

MartijnVisser requested a review from tisonkun July 12, 2022 13:31

tisonkun reviewed Jul 14, 2022

View reviewed changes

imaffe reviewed Jul 25, 2022

View reviewed changes

Comment thread ...n/java/org/apache/flink/connector/pulsar/source/enumerator/assigner/SharedSplitAssigner.java

imaffe reviewed Jul 25, 2022

View reviewed changes

Comment thread ...n/java/org/apache/flink/connector/pulsar/source/enumerator/assigner/NormalSplitAssigner.java Outdated

imaffe reviewed Jul 25, 2022

View reviewed changes

Comment thread ...n/java/org/apache/flink/connector/pulsar/source/enumerator/assigner/SharedSplitAssigner.java

imaffe reviewed Jul 25, 2022

View reviewed changes

Comment thread ...n/java/org/apache/flink/connector/pulsar/source/enumerator/assigner/NormalSplitAssigner.java Outdated

syhily force-pushed the feature/new-seek-lifecycle branch from 12049bf to 21eda6f Compare July 28, 2022 08:22

syhily requested review from a49a and imaffe and removed request for imaffe August 11, 2022 19:16

syhily force-pushed the feature/new-seek-lifecycle branch from 2baa322 to f807c4a Compare August 12, 2022 03:24

tisonkun reviewed Aug 12, 2022

View reviewed changes

Comment thread ...org/apache/flink/connector/pulsar/source/reader/split/PulsarOrderedPartitionSplitReader.java

tisonkun requested review from tisonkun and removed request for a49a August 12, 2022 03:38

syhily force-pushed the feature/new-seek-lifecycle branch from f807c4a to 938be57 Compare August 12, 2022 05:18

syhily added 4 commits August 12, 2022 15:31

[FLINK-27399][Connector/Pulsar] Change the initial start cursor and s…

085c3d2

…top cursor for better handle the consuming behaviors.

[FLINK-27399][Connector/Pulsar] Create the initial subscription inste…

ff3cc72

…ad seek every time. This should fix the wrong position setting.

[FLINK-27399][Connector/Pulsar] Fix the wrong stop cursor, make sure …

ce48d74

…it stops at correct space

[FLINK-27399][Connector/Pulsar] Drop Consumer.seek() for apache/pulsa…

7ea6b3c

…r#16171

syhily force-pushed the feature/new-seek-lifecycle branch from 938be57 to 7ea6b3c Compare August 12, 2022 07:31

tisonkun approved these changes Aug 12, 2022

View reviewed changes

tisonkun merged commit 18d21a0 into apache:master Aug 12, 2022

syhily deleted the feature/new-seek-lifecycle branch August 13, 2022 06:15

syhily mentioned this pull request Aug 13, 2022

[BP-1.15][FLINK-27399][Connector/Pulsar] [Connector/Pulsar] Change initial consuming position setting logic for better handle the checkpoint. #20564

Merged

syhily mentioned this pull request Aug 13, 2022

[BP-1.14][FLINK-27399][Connector/Pulsar] [Connector/Pulsar] Change initial consuming position setting logic for better handle the checkpoint. #20565

Merged

syhily mentioned this pull request Aug 15, 2022

[Test][FLINK-28352] [Umbrella] Make Pulsar connector stable on Flink CI streamnative/flink#181

Closed

15 tasks

imaffe mentioned this pull request Aug 19, 2022

[backport] Backport upcoming PRs to apache/flink back to streamnative/flink streamnative/flink#192

Closed

Conversation

syhily commented Jun 15, 2022 • edited by tisonkun Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

flinkbot commented Jun 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

Uh oh!

imaffe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MartijnVisser commented Jul 11, 2022

Uh oh!

MartijnVisser commented Jul 12, 2022

Uh oh!

tisonkun commented Jul 12, 2022

Uh oh!

tisonkun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MartijnVisser commented Jul 21, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

syhily commented Aug 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

syhily commented Jun 15, 2022 •

edited by tisonkun

Loading

flinkbot commented Jun 15, 2022 •

edited

Loading

tisonkun left a comment •

edited

Loading