flink icon indicating copy to clipboard operation
flink copied to clipboard

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

Open syhily opened this issue 2 years ago • 5 comments

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)

syhily avatar Jun 15 '22 16:06 syhily

CI report:

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

flinkbot avatar Jun 15 '22 16:06 flinkbot

@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 avatar Jul 11 '22 07:07 MartijnVisser

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

MartijnVisser avatar Jul 12 '22 13:07 MartijnVisser

@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 avatar Jul 12 '22 13:07 tisonkun

@syhily Can you resolve the latest review comments?

MartijnVisser avatar Jul 21 '22 12:07 MartijnVisser

@tisonkun Finally, the ci turns green.

syhily avatar Aug 12 '22 11:08 syhily