+1 for xyz.[min|max]; imo it becomes obvious if think of it like a yaml file:

xyz:
    min:
    max:

opposed to

min-xyz:
max-xyz:

IIRC this would also be more in-line with the hierarchical scheme for config options we decided on months ago.

On 27/04/2020 13:25, Xintong Song wrote:
+1 for Robert's idea about adding tests/tools checking the pattern of new
configuration options, and migrate the old ones in release 2.0.

Concerning the preferred pattern, I personally agree with Till's opinion. I
think 'xyz.[min|max]' somehow expresses that 'min' and 'max' are properties
of 'xyz', while 'xyz' may also have other properties. An example could be
'taskmanager.memory.network.[min|max|fraction]'.

Thank you~

Xintong Song



On Mon, Apr 27, 2020 at 6:00 PM Till Rohrmann <trohrm...@apache.org> wrote:

Hi everyone,

as Robert said I think the problem is that we don't have strict guidelines
and every committer follows his/her personal taste. I'm actually not sure
whether we can define bullet-proof guidelines but we can definitely
make them more concrete.

In this case here, I have to admit that I have an opposing opinion/taste. I
think it would be better to use xyz.min and xyz.max. The reason is that we
configure a property of xyz which consists of the minimum and maximum
value. Differently said, min and max belong semantically together and hence
should be defined together. You can think of it as if the type of the xyz
config option would be a tuple of two integers instead of two individual
integers.

A comment concerning the existing styles of config options: I think many of
the config options which follow the max-xzy pattern are actually older
configuration options.

Cheers,
Till

On Mon, Apr 27, 2020 at 10:34 AM Robert Metzger <rmetz...@apache.org>
wrote:

Thanks for starting this discussion.
I believe the different options are a lot about personal taste, there are
no objective arguments why one option is better than the other.

I agree with your proposal to simply go with the "max-xyz" pattern, as
this
is the style of the majority of the current configuration options in
Flink
(maybe this also means it is the taste of the majority of Flink
developers?).

I would propose to add a test or some tooling that checks that all new
configuration parameters follow this pattern, as well as tickets for
Flink
2.0 to migrate the "wrong" configuration options.



On Wed, Apr 22, 2020 at 5:47 AM Yangze Guo <karma...@gmail.com> wrote:

Hi, everyone,

I'm working on FLINK-16605 Add max limitation to the total number of
slots[1]. In the PR, I, Gary and Xintong has a discussion[2] about the
config option of this limit.
The central question is whether the "max" should be part of the
hierarchy or part of the property itself.

It means there could be two patterns:
- max-xyz
- xyz.max

Currently, there is no clear consensus on which style is better and we
could find both patterns in the current Flink. Here, I'd like to first
sort out[3]:

Config options follow the "max-xyz" pattern:
- restart-strategy.failure-rate.max-failures-per-interval
- yarn.maximum-failed-containers
- state.backend.rocksdb.compaction.level.max-size-level-base
- cluster.registration.max-timeout
- high-availability.zookeeper.client.max-retry-attempts
- rest.client.max-content-length
- rest.retry.max-attempts
- rest.server.max-content-length
- jobstore.max-capacity
- taskmanager.registration.max-backoff
- compiler.delimited-informat.max-line-samples
- compiler.delimited-informat.min-line-samples
- compiler.delimited-informat.max-sample-len
- taskmanager.runtime.max-fan
- pipeline.max-parallelism
- execution.checkpointing.max-concurrent-checkpoint
- execution.checkpointing.min-pause

Config options follow the "xyz.max" pattern:
- taskmanager.memory.jvm-overhead.max
- taskmanager.memory.jvm-overhead.min
- taskmanager.memory.network.max
- taskmanager.memory.network.min
- taskmanager.network.request-backoff.max
- env.log.max

Config options do not follow the above two patterns:
- akka.client-socket-worker-pool.pool-size-max
- akka.client-socket-worker-pool.pool-size-min
- akka.fork-join-executor.parallelism-max
- akka.fork-join-executor.parallelism-min
- akka.server-socket-worker-pool.pool-size-max
- akka.server-socket-worker-pool.pool-size-min
- containerized.heap-cutoff-min
- blob.offload.minsize

It seems more config options follow the "max-xyz" pattern. From my
side, I do not have a strong preference. But it probably make sense to
follow one of them in Flink.
If we decide to make it a naming convention and align all config
options to it, I prefer to follow the "max-xyz" pattern to minimize
the infect to user.

Looking forward to your feedback!

[1] https://issues.apache.org/jira/browse/FLINK-16605
[2] https://github.com/apache/flink/pull/11615#discussion_r412316648
[3]

https://ci.apache.org/projects/flink/flink-docs-release-1.10/ops/config.html
Best,
Yangze Guo


Reply via email to