-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-15741][docs][TTL] Fix TTL docs after enabling RocksDB compaction filter by default #10934
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
Conversation
7ac3802 to
04f0541
Compare
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 7ac3802 (Thu Jan 23 14:50:39 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. DetailsThe 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:
|
GJL
left a comment
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.
It's ok. It's up to you whether you want to address my suggestions.
docs/dev/stream/state/state.md
Outdated
| Updating the timestamp more often can improve cleanup speed | ||
| but it decreases compaction performance because it uses JNI call from native code. | ||
| If you enable the default background cleanup then this strategy will be activated for RocksDB backend and the current timestamp will be queried each time 1000 entries have been processed. | ||
| The default background cleanup for RocksDB backend queries the current timestamp each time when 1000 entries have been processed. |
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.
Imo, "when" is not needed.
Maybe another alternative that sounds better to me: [...] queries the current timestamp for every 1000 processed entries
docs/dev/stream/state/state.md
Outdated
| ##### Cleanup during RocksDB compaction | ||
|
|
||
| If RocksDB state backend is used, another cleanup strategy is to activate Flink specific compaction filter. | ||
| If RocksDB state backend is used, another cleanup strategy is to call Flink specific compaction filter. |
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.
Currently it sounds like that the cleanup strategy won't be active by default or there are other strategies that can be used – Maybe that's just me. My proposal would be:
If the RocksDB state backend is used, a Flink specific compaction filter will be employed to enable background cleanup.
…on filter by default
04f0541 to
1ce907b
Compare
|
Thanks for the review @GJL |
What is the purpose of the change
RocksDB compaction filter is always enabled by default after #10866 and we deprecated its disabling. The docs should not refer to its enabling/disabling. The Chinese translation still has to be adjusted separately.
Brief change log
adjust
state.mdVerifying this change
doc change
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation