Thanks for driving the discussion. +1 for the proposal and +1 for the `InferMode.NONE` option.
Best, Lijie Ron liu <ron9....@gmail.com> 于2024年4月18日周四 11:36写道： > Hi, Xia > > Thanks for driving this FLIP. > > This proposal looks good to me overall. However, I have the following minor > questions: > > 1. FLIP introduced `table.exec.hive.infer-source-parallelism.mode` as a new > parameter, and the value is the enum class `InferMode`, I think the > InferMode class should also be introduced in the Public Interfaces section! > 2. You mentioned in FLIP that the default value of > `table.exec.hive.infer-source-parallelism.max` is 1024, I checked through > the code that the default value is 1000? > 3. I also agree with Muhammet's idea that there is no need to introduce the > option `table.exec.hive.infer-source-parallelism.enabled`, and that > expanding the InferMode values will fulfill the need. There is another > issue to consider here though, how are > `table.exec.hive.infer-source-parallelism` and > `table.exec.hive.infer-source-parallelism.mode` compatible? > 4. In FLIP-367 it is supported to be able to set the Source's parallelism > individually, if in the future HiveSource also supports this feature, > however, the default value of > `table.exec.hive.infer-source-parallelism.mode` is `InferMode. DYNAMIC`, at > this point will the parallelism be dynamically derived or will the manually > set parallelism take effect, and who has the higher priority? > > Best, > Ron > > Xia Sun <xingbe...@gmail.com> 于2024年4月17日周三 12:08写道： > > > Hi Jeyhun, Muhammet, > > Thanks for all the feedback! > > > > > Could you please mention the default values for the new configurations > > > (e.g., table.exec.hive.infer-source-parallelism.mode, > > > table.exec.hive.infer-source-parallelism.enabled, > > > etc) ? > > > > > > Thanks for your suggestion. I have supplemented the explanation regarding > > the default values. > > > > > Since we are introducing the mode as a configuration option, > > > could it make sense to have `InferMode.NONE` option also? > > > The `NONE` option would disable the inference. > > > > > > This is a good idea. Looking ahead, it could eliminate the need for > > introducing > > a new configuration option. I haven't identified any potential > > compatibility issues > > as yet. If there are no further ideas from others, I'll go ahead and > update > > the FLIP to > > introducing InferMode.NONE. > > > > Best, > > Xia > > > > Muhammet Orazov <mor+fl...@morazow.com.invalid> 于2024年4月17日周三 10:31写道： > > > > > Hello Xia, > > > > > > Thanks for the FLIP! > > > > > > Since we are introducing the mode as a configuration option, > > > could it make sense to have `InferMode.NONE` option also? > > > The `NONE` option would disable the inference. > > > > > > This way we deprecate the `table.exec.hive.infer-source-parallelism` > > > and no additional `table.exec.hive.infer-source-parallelism.enabled` > > > option is required. > > > > > > What do you think? > > > > > > Best, > > > Muhammet > > > > > > On 2024-04-16 07:07, Xia Sun wrote: > > > > Hi everyone, > > > > I would like to start a discussion on FLIP-445: Support dynamic > > > > parallelism > > > > inference for HiveSource[1]. > > > > > > > > FLIP-379[2] has introduced dynamic source parallelism inference for > > > > batch > > > > jobs, which can utilize runtime information to more accurately decide > > > > the > > > > source parallelism. As a follow-up task, we plan to implement the > > > > dynamic > > > > parallelism inference interface for HiveSource, and also switch the > > > > default > > > > static parallelism inference to dynamic parallelism inference. > > > > > > > > Looking forward to your feedback and suggestions, thanks. > > > > > > > > [1] > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-445%3A+Support+dynamic+parallelism+inference+for+HiveSource > > > > [2] > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-379%3A+Dynamic+source+parallelism+inference+for+batch+jobs > > > > > > > > Best regards, > > > > Xia > > > > > >
