[GitHub] flink issue #5582: [FLINK-8790][State] Improve performance for recovery from...
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5582 LGTM ð Very nice work. I will merge it with some very minor touchups. ---
[GitHub] flink issue #5582: [FLINK-8790][State] Improve performance for recovery from...
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5582 Hi @StefanRRichter I updated the PR according to the previous discussions, could you please have a look when you have time? The travis failed is unrelated, it's a checkstyle error introduced by the previous PRs. ---
[GitHub] flink issue #5582: [FLINK-8790][State] Improve performance for recovery from...
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5582 @StefanRRichter Thanks for your nice review and preventing this PR to fall into a sick way, I will change the code according to your comments and ping you again when I finish this. ---
[GitHub] flink issue #5582: [FLINK-8790][State] Improve performance for recovery from...
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5582 @StefanRRichter Thanks for your nice review, addressed your comments, could you please have a look again? ---
[GitHub] flink issue #5582: [FLINK-8790][State] Improve performance for recovery from...
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5582 Hi @StefanRRichter could you please have a look at this? ---
[GitHub] flink issue #5582: [FLINK-8790][State] Improve performance for recovery from...
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5582 Unfortunately, after confirming with RocksDB, the `deleteRange()` is still an experimental feature, it may have impact on read performance currently(event thought we could use the ReadOption to reduce the impaction). In practice, I tested the impact of read performance of `deleteRange()` in our case (only delete 2 ranges at most), I didn't find any impact in fact. And the TiKV has already used it to delete entire shards. But, to be on the safe side, I think the current PR should be frozen, but I think the implementation base on `deleteRange()` in this PR should be a better implementation(especially when user scaling up the job, in that case we only need to clip the RocksDB without iterating any records, a super fast way) if the `deleteRange()` is no longer a feature of experimental. Anyways, even although we can't use the `deleteRange()` currently, but we can still improve the performance of the incremental checkpoint in somehow. We can improve it the by the follow way: if one of the state handle's key-group is a sub-range of the target key-group range. we can open it directly to prevent the overhead of iterating it. @StefanRRichter What do you think? If you don't object this, I will update the PR follow the above approach. ---
[GitHub] flink issue #5582: [FLINK-8790][State] Improve performance for recovery from...
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5582 Thanks, looking forward. ---
[GitHub] flink issue #5582: [FLINK-8790][State] Improve performance for recovery from...
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5582 Thanks for the contribution! We are currently busy with the 1.5 release. I will have a closer look at this PR and your other pending JIRAs after the release is out. ---
[GitHub] flink issue #5582: [FLINK-8790][State] Improve performance for recovery from...
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5582 @StefanRRichter Could you please have a look at this? ---
