wsry commented on pull request #13595: URL: https://github.com/apache/flink/pull/13595#issuecomment-716275114
> Thanks a lot, the code looks generally good to me. > > I think the current configuration works also as a feature flag, which is nice. If I read it correctly, sort-shuffle will start from a parallelism of INT_MAX, which means it is never used unless users explicitly configure it to be used. So we do not need any additional feature flag. > > What confused my were the minimum and maximum buffer requirements. > > * There is a maximum number of buffer (`taskmanager.network.sort-merge-blocking-shuffle.max-buffers-per-partition`). Why do we need that? What would happen if there were more buffers? > * We also still have a minimum of `numSubpartitions + 1` buffers. Is this a requirement for the write-phase, or only for the read phase? As I understand it, a part of the motivation of this sort-nased shuffle is to decouple the number of needed memory buffers from the parallelism. Could we drop this minimum? @StephanEwen 1. As suggested by @zhijiangW Zhijiang in the discussion thread, I replace `taskmanager.network.sort-shuffle.min-parallelism` with a feature flag `taskmanager.network.sort-shuffle.enabled` for ease of use. 2. You are right, the motivation of `taskmanager.network.sort-shuffle.max-buffers` is used to decouple the number of needed memory buffers from the parallelism. So it is the upper bound of required memory, if `numSubpartitions + 1` is greater than this config value, this config value is used. The config option is currently used by write-phase, after we implement partition request restriction or buffer management in FLINK-16641, this option can also be reused by read phase. Then we can totally decouple the number of needed memory buffers from the parallelism for both write-phase and read-phase. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
