hwanju commented on a change in pull request #14499:
URL: https://github.com/apache/flink/pull/14499#discussion_r555765193
##########
File path:
flink-core/src/main/java/org/apache/flink/configuration/ClusterOptions.java
##########
@@ -88,21 +93,28 @@
.build());
@Documentation.Section(Documentation.Sections.EXPERT_CLUSTER)
- public static final ConfigOption<Boolean> HALT_ON_FATAL_ERROR =
- key("cluster.processes.halt-on-fatal-error")
+ public static final ConfigOption<Boolean> HALT_ON_SYSTEM_EXIT =
+ key("cluster.processes.halt-on-system-exit")
Review comment:
>Renaming an existing parameter has a bigger impact on users as they
possibly have to change their existing configuration files when upgrading.
This was Robert's suggestion and I also had the same concern. But I
incorporated that suggestion because this flag seems to be more of advanced
expert feature, which may be used by a few individuals, and I see sometimes
Flink config names have been changed slightly or largely. However, as we do not
know if this feature is used much, I am fine to keep the name same. I am
willing to follow votes.
> would be ok with that change if we decide to merge both parameters into
one as they are more or less just defining some special exit behavior. What do
you think?
Like your following comment, user system exit vs. global system exit
behaviors need to be separate for clarity. Those can be under the same
`ClusterOptions` but I am still leaning toward having separate configurations.
Having said that, the only alternative we can think is just to keep
`cluster.process.halt-on-fatal-error` as is. What do you think @rmetzger ?
----------------------------------------------------------------
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:
[email protected]
