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

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

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


   
   > > 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.
   
   Your understanding is correct.
   
   
   > > 
   > >     * This currently only works for StreamTasks? What about batch tasks? We should implement this in a way that it works for all user code.
   
   > As discussed in the Jira issue, we start with partial coverage, after which more coverage can be done incrementally with separate tickets. If I move monitoring into Task.doRun, which I will do, we can effectively monitor more coverage not just stream task.
   
   I believe that this is a fair approach! We just need to make sure that people don't have the wrong expectation from this pull request. 
   


----------------------------------------------------------------
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