flink icon indicating copy to clipboard operation
flink copied to clipboard

[FLINK-28732][state] Deprecate ambiguous StateTTLConfig#cleanFullSnapshot API

Open Myasuka opened this issue 2 years ago • 7 comments

What is the purpose of the change

This PR deprecates the ambiguous StateTTLConfig#cleanFullSnapshot API with the newly introduced #cleanupOnFullScanSnapshot API.

Brief change log

  1. Deprecate the StateTTLConfig#cleanFullSnapshot API
  2. Introduce StateTTLConfig#cleanupOnFullScanSnapshot API
  3. Add docs to describe the difference.

Verifying this change

This change is already covered by existing tests.

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 introduce a new feature? no
  • If yes, how is the feature documented? no

Myasuka avatar Aug 02 '22 10:08 Myasuka

CI report:

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

flinkbot avatar Aug 02 '22 10:08 flinkbot

@pnowojski , @rkhachatryan could you please take a look at this?

Myasuka avatar Aug 06 '22 11:08 Myasuka

@rkhachatryan do you have more concerns? My private build is successful: https://dev.azure.com/myasuka/flink/_build/results?buildId=429&view=results.

Myasuka avatar Aug 09 '22 12:08 Myasuka

I'm concerned about introducing the unnecessary term (from my perspective), but I wouldn't mind merging this PR as it is.

rkhachatryan avatar Aug 09 '22 12:08 rkhachatryan

Would you please approve this PR @rkhachatryan if no more concerns?

Myasuka avatar Aug 09 '22 13:08 Myasuka

I still have the same concern. Probably someone else would disagree with me and approve the PR, I'd be absolutely fine with that.

rkhachatryan avatar Aug 09 '22 13:08 rkhachatryan

I still have the same https://github.com/apache/flink/pull/20421#discussion_r940348087. Probably someone else would disagree with me and approve the PR, I'd be absolutely fine with that.

It was too close to the feature freeze date to ask someone else to review, I think we should make the code clean instead of rushing the changes. I would not drive this PR merged in 1.16 with existing concerns.

Myasuka avatar Aug 09 '22 14:08 Myasuka