Skip to content

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

Merged
tisonkun merged 1 commit into
apache:release-1.14from
streamnative:backport/release-1.14/FLINK-27399
Aug 14, 2022
Merged

[BP-1.14][FLINK-27399][Connector/Pulsar] [Connector/Pulsar] Change initial consuming position setting logic for better handle the checkpoint.#20565
tisonkun merged 1 commit into
apache:release-1.14from
streamnative:backport/release-1.14/FLINK-27399

Conversation

@syhily

@syhily syhily commented Aug 13, 2022

Copy link
Copy Markdown
Contributor

This is a backport PR for #19972.

@flinkbot

flinkbot commented Aug 13, 2022

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@syhily syhily force-pushed the backport/release-1.14/FLINK-27399 branch from 81e7328 to 04dfdd4 Compare August 13, 2022 18:48

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for cherry-picking this patch. One comment is inline.

Also, I'd like to confirm that you bring the test utils from master to 1.14 for Pulsar Connector tests. Is it right?


<properties>
<pulsar.version>2.8.0</pulsar.version>
<pulsar.version>2.9.1</pulsar.version>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How is this version bump necessary?

Basically, we should not bump version in a patch version, especially which can introduce new features.

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.

We have to use this version bump because the underlying API is introduced on Pulsar 2.9.x.

@tisonkun tisonkun requested a review from MartijnVisser August 14, 2022 02:08
@syhily

syhily commented Aug 14, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for cherry-picking this patch. One comment is inline.

Also, I'd like to confirm that you bring the test utils from master to 1.14 for Pulsar Connector tests. Is it right?

Yep. The testing tools are introduced for adding transaction support for mocked Pulsar.

@syhily

syhily commented Aug 14, 2022

Copy link
Copy Markdown
Contributor Author

@PatrickRen Can you help me review this backport issue?

…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
@syhily syhily force-pushed the backport/release-1.14/FLINK-27399 branch from 04dfdd4 to f9666b8 Compare August 14, 2022 06:20

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Merging...

Anyone can leave a review comment after merge.

@tisonkun tisonkun merged commit 95d14ed into apache:release-1.14 Aug 14, 2022
@syhily syhily deleted the backport/release-1.14/FLINK-27399 branch August 14, 2022 15:01
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