[GitHub] [flink] hwanju commented on pull request #14499: [FLINK-15156] Warn user if System.exit() is called in user code

[GitBox <gi...@apache.org>]

hwanju commented on pull request #14499:
URL: https://github.com/apache/flink/pull/14499#issuecomment-754770469


   Thanks for the review! I've mostly addressed @XComp but we seem to agree on unifying security managers, so I should work on it along with changed configurations.
   
   > Thanks a lot for this PR. Here are some suggestions on the config parameters and the overall SecurityManager use in Flink:
   > 
   >     * Only set security manager if `cluster.processes.halt-on-system-exit == true && cluster.intercept-system-exit == DISABLED` (the defaults) ... to avoid performance penalties for users who don't need this.
   
   IIUC, either halt-on-system-exit is set to true or intercept-system-exit is not disabled, security manager is set. The former is applied globally regardless of user vs. framework, while the latter is only applied for user-monitored section.
   
   > 
   >     * Rename `cluster.processes.halt-on-fatal-error` to `cluster.processes.halt-on-system-exit` . This configuration option globally changes the behavior from exit to halt. If configured to true, always halt() instead of system.exit. Remove ExitTrappingSecurityManager.
   > 
   >     * Rename `security.check-system-exit` to `cluster.intercept-user-system-exit` (options: disabled, log, throw).
   > 
   >     * Rename `FlinkUserSecurityManager` to `FlinkSecurityManager`.
   > 
   > 
   > ... I'm concerned that the PR as-is makes the codebase unnecessarily complicated (with two security managers overwriting each other (`cluster.processes.halt-on-fatal-error` has no effect anymore))
   > 
   > cc @mxm, if you want to contribute to this discussion.
   
   Will unify those into FlinkSecurityManager.


----------------------------------------------------------------
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:
users@infra.apache.org