-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-17376][API/DataStream]Deprecated methods and related code updated #12189
Conversation
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 256cc76 (Sat May 16 11:41:48 UTC 2020) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
@@ -388,7 +389,7 @@ public void initializeState(FunctionInitializationContext context) throws Except | |||
} | |||
|
|||
OperatorStateStore stateStore = context.getOperatorStateStore(); | |||
restoredBucketStates = stateStore.getSerializableListState("bucket-states"); | |||
this.restoredBucketStates = stateStore.getListState(new ListStateDescriptor("bucket-states", State.class)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not actually work because getListState(..., State.class)
should be created a GenericTypeInfo
, which is incompatible with the JavaSerializer
used before internally here.
@flinkbot run azure |
Sorry for delay. I am stuck with some issues, and plan to work on it this weekend. If its needed urgently, I am fine with someone else taking it up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mghildiy thanks for the contribution. It seems there still some other FoldingState
usages are not touched in current change.
flink-runtime/src/main/java/org/apache/flink/runtime/state/DefaultOperatorStateBackend.java
Show resolved
Hide resolved
We do this because we want to deprecate that method. We will have to get rid of using JavaSerialization completely soon, though.
@aljoscha Will check it now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still some places contain folding state, such as TtlFoldingStateVerifier
, TestSharedBuffer
, KeyedStateStore
, DefaultKeyedStateStore
, WindowOperator
, WindowOperatorContractTest
.
and we need to update the doc state.md
and state.zh.md
@klion26 That's a good point. This PR/Jira issue is about removing the access methods, we can remove Good point about updating the docs! I will do that |
This code was using the deprecated getSerializedListState(), which we want to remove.
@klion26 I pushed some more changes. Please take another look. |
@aljoscha thanks for the update. Remove diff for |
pushed changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aljoscha thanks for the work. LGTM now
It is probably worthwhile mentioning the transitive effect of these changes in the release notes. |
That's a very good point! I'm adding a release note. |
What is the purpose of the change
Objective of this PR is to remove the following deprecated method and update related code:
Brief change log
NA
Verifying this change
NA
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (yes / no)Documentation