Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[FLINK-34987][state] Introduce Internal State for Async State API #24651

Closed
wants to merge 3 commits into from
Closed

[FLINK-34987][state] Introduce Internal State for Async State API #24651

wants to merge 3 commits into from

Conversation

masteryhx
Copy link
Contributor

What is the purpose of the change

Introduce Internal State to delegates all state requests to AEC.

Brief change log

  • Introduce new StateDescriptor for Async State API
  • Introduce InternalKvState for Async State API
  • Implement Async Value State

Verifying this change

This change added tests and can be verified as follows:

  • Added StateDescriptorTest
  • Modified existed AsyncExecutionControllerTest

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

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

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@flinkbot
Copy link
Collaborator

flinkbot commented Apr 11, 2024 •
edited
Loading

CI report:

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

Zakelly
Zakelly reviewed Apr 11, 2024
View reviewed changes
Copy link
Contributor

@Zakelly Zakelly left a comment

Choose a reason for hiding this comment

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

Thanks for this PR!
I left some comments PTAL.

flink-runtime/src/main/java/org/apache/flink/runtime/state/v2/internal/InternalKvState.java Outdated Show resolved Hide resolved
flink-runtime/src/main/java/org/apache/flink/runtime/state/v2/internal/InternalKvState.java Outdated Show resolved Hide resolved
flink-runtime/src/main/java/org/apache/flink/runtime/state/v2/internal/InternalKvState.java Outdated
public StateDescriptor<V> getStateDescriptor() {
return stateDescriptor;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we provide serializer-related interfaces under this class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a method of value serializer and related test.

flink-runtime/src/main/java/org/apache/flink/runtime/state/v2/internal/ValueStateImpl.java Outdated Show resolved Hide resolved
@masteryhx masteryhx changed the title [FLINK-35045][state] Introduce Internal State for Async State API [FLINK-34987][state] Introduce Internal State for Async State API Apr 12, 2024
Zakelly
Zakelly reviewed Apr 15, 2024
View reviewed changes
Copy link
Contributor

@Zakelly Zakelly left a comment

Choose a reason for hiding this comment

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

Thanks for the update! I leave some further comments here.

flink-runtime/src/main/java/org/apache/flink/runtime/state/v2/StateDescriptor.java Outdated Show resolved Hide resolved
flink-runtime/src/main/java/org/apache/flink/runtime/state/v2/InternalKeyedState.java Outdated Show resolved Hide resolved
flink-runtime/src/main/java/org/apache/flink/runtime/state/v2/StateDescriptor.java Outdated Show resolved Hide resolved
flink-runtime/src/main/java/org/apache/flink/runtime/state/v2/ValueStateImpl.java Outdated Show resolved Hide resolved
fredia
fredia reviewed Apr 15, 2024
View reviewed changes
Copy link
Contributor

@fredia fredia left a comment

Choose a reason for hiding this comment

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

@masteryhx Thanks for the PR, I left some comments, PTAL

flink-runtime/src/main/java/org/apache/flink/runtime/state/v2/StateDescriptor.java Show resolved Hide resolved
flink-runtime/src/main/java/org/apache/flink/runtime/state/v2/InternalKeyedState.java Show resolved Hide resolved
Zakelly
Zakelly approved these changes Apr 16, 2024
View reviewed changes
Copy link
Contributor

@Zakelly Zakelly left a comment

Choose a reason for hiding this comment

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

Thanks for the update LGTM

@masteryhx
Copy link
Contributor Author

Rebased master.
I will merge it after the CI is green.

@masteryhx
Copy link
Contributor Author

@flinkbot run azure

@masteryhx
Copy link
Contributor Author

Rebased again.

masteryhx added a commit that referenced this pull request Apr 17, 2024
@masteryhx
[FLINK-34987][state] Implement Async Value State (#24651)
2c5078b
@masteryhx masteryhx closed this Apr 17, 2024
@masteryhx masteryhx deleted the FLINK-34987 branch April 17, 2024 14:32
hanyuzheng7 pushed a commit to hanyuzheng7/flink that referenced this pull request May 6, 2024
@masteryhx @hanyuzheng7
[FLINK-34987][state] Implement Async Value State (apache#24651)
0c0eaef
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fredia fredia fredia left review comments

@Zakelly Zakelly Zakelly approved these changes

Assignees
No one assigned
Labels
component=Runtime/StateBackends
Projects
None yet
Milestone
No milestone
4 participants
@masteryhx @flinkbot @Zakelly @fredia