[FLINK-12212] [docs] clarify that operator state is checkpointed asynchronously #8185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community 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:
|
StefanRRichter
requested changes
Apr 17, 2019
There was a problem hiding this comment.
@knaufk thanks for improving this documentation. I had two comments, otherwise this looks very good. We can merge once the comments are addressed.
docs/ops/state/large_state_tuning.md
Outdated
| To get state to be snapshotted asynchronously, applications have to do two things: | ||
| For state to be snapshotted asynchronsously, you need to use a state backend which supports asynchronous snapshotting. | ||
| Starting from Flink 1.3, both RocksDB-based as well as heap-based state backends (`filesystem`) support asynchronous | ||
| snapshotting and use it by default. This applies to to both managed operator state as well as managed keyed state. |
There was a problem hiding this comment.
We don't talk about timer state here or do you count that under "keyed"?
docs/ops/state/large_state_tuning.md
Outdated
| fully asynchronous snapshots. Starting from Flink 1.3, heap-based state backends also support asynchronous snapshots. | ||
|
|
||
| The above two points imply that large state should generally be kept as keyed state, not as operator state. | ||
| <span class="label label-info">Note</span> *The combination RocksDB state backend / with incremental checkpoint / with heap-based timers currently does NOT support asynchronous snapshots for the timers state. |
There was a problem hiding this comment.
I would correct this: in fact the combination rocks state + heap timers already leads to sync snapshots for the timer part. not just for incremental.
|
@StefanRRichter Thanks for the review. I have addressed both comments. |
tianchen92
pushed a commit
to tianchen92/flink
that referenced
this pull request
May 13, 2019
…hronously This closes apache#8185.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
Brief change log
see above
Verifying this change
N/A
Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation