apache
[FLINK-36149][state] Make the RocksdbCompactFilter parameters take effect directly at the statebackend level
What is the purpose of the change
In Rank operators, add the cleanupInRocksdbCompactFilter(queryTimeAfterNumEntries) parameter setting, allowing adjustment based on the size of user state to enhance performance.
Brief change log
- Add a configuration parameter, TABLE_EXEC_RANK_ROCKSDB_CLEANUP_QUERY_TIME_AFTER_NUM_ENTRIES, in StreamExecRank.
- Update StateConfigUtil to support creating StateTtlConfig using retentionTime and queryTimeAfterNumEntries.
- Support configuration of cleanupFullSnapshot and cleanupInRocksdbCompactFilter.
Verifying this change
This change is already covered by existing tests, such as AppendOnlyFirstNFunctionTest、TopNFunctionTestBase.
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): (no) - 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? (not documented)
CI report:
- 8cc36d1e1d2fe8add0ccee910efd573abd2221db Azure: SUCCESS
Bot commands
The @flinkbot bot supports the following commands:@flinkbot run azurere-run the last Azure build
cc @Zakelly @masteryhx since you're the expert in this area, would one of you help the review if you have time？
@lexluo09 thanks for contributing this! Since the scope of the pr changes, the module name in the commit msg should be updated as well(
table->state).
Thank you for helping with the review. I have made the necessary changes based on your feedback.
And BTW, you should run mvn package -Dgenerate-config-docs -pl flink-docs -am -nsu -DskipTests -Pskip-webui-build if you add/change the configurations. Please check the instructions in flink-docs/README.md.
And BTW, you should run
mvn package -Dgenerate-config-docs -pl flink-docs -am -nsu -DskipTests -Pskip-webui-buildif you add/change the configurations. Please check the instructions inflink-docs/README.md.
@Zakelly Thank you for your suggestion. I have executed the command to generate the documentation for the configuration as per your recommendation.
The CI failed. Would you mind fixing this? @Zakelly Thank you for your suggestion. The CI has been successfully fixed.
@lexluo09 Sorry for the late reply, overall looks good. Only one minor thing:
Thank you for taking the time to help review the code. It has been fixed now.
@Zakelly
I need help confirming a question: In ProtoUtils, the builder.cleanupInRocksdbCompactFilter only sets queryTimeAfterNumEntries and does not set periodicCompactionTime. Should we handle this situation for compatibility? @Zakelly
@lexluo09
Yes, we should keep compatibility for that. I think in current PR, if it does builder.cleanupInRocksdbCompactFilter(rocksdbCompactFilterCleanupStrategyProto.getQueryTimeAfterNumEntries());, the DEFAULT_PERIODIC_COMPACTION_TIME still takes effect, aka 30 days set. So no compatibility issues, right?
Yes, we should keep compatibility for that. I think in current PR, if it does
builder.cleanupInRocksdbCompactFilter(rocksdbCompactFilterCleanupStrategyProto.getQueryTimeAfterNumEntries());, theDEFAULT_PERIODIC_COMPACTION_TIMEstill takes effect, aka 30 days set. So no compatibility issues, right?
Yes, I also think this is reasonable. The DEFAULT_PERIODIC_COMPACTION_TIME will still be retained.
@Zakelly Thank you for taking the time to review. I have made all the necessary changes. When you have the time, please kindly take another look. Thank you for your assistance.
@Zakelly The CI tests have passed. When you have time, could you please take a look? Thank you for your assistance.